This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
use initgroups_dyn-less nss impls in a thread-safer way
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: libc-alpha at sources dot redhat dot com
- Date: Fri, 20 Jan 2012 01:40:05 -0200
- Subject: use initgroups_dyn-less nss impls in a thread-safer way
AFAICT, nscd may call getgrouplist() from multiple threads
simultaneously.
This is fine for all (?) nss implementations provided by glibc itself,
for they supply a thread-safe implementation of the inigroups_dyn entry
point.
However, for other nss impementations that don't, as well as for older
versions of glibc that didn't have _nss_files_initgroups_dyn, this means
nscd threads may interfere with one another as they iterate over the
group list with the not-really-reentrant getgrent_r, so that they get
non-overlapping subsets of the group entries.
I realize this might not be the best place for this fix; arguably, nscd
should avoid concurrency issues when using such not-really-reentrant
interfaces like any other program, but since its whole point is to
improve performance and holding an unnecessary lock around every
getgrouplist call would hurt it, I propose a change that will have a
minimal impact that may be easily avoided by introducing an
initgroups_dyn entry point, and that will increase the odds of
correctness for general uses that don't take care of locking
getgrouplist() themselves (as long as they don't getgrent*
concurrently).
Is this ok?
for ChangeLog
2012-01-19 Alexandre Oliva <aoliva@redhat.com>
* grp/compat-initgroups.c (compat_call): Avoid concurrency
between setgrent and endgrent.
--- grp/compat-initgroups.c
+++ grp/compat-initgroups.c
@@ -1,3 +1,5 @@
+#include <bits/libc-lock.h>
+
/* Prototype for the setgrent functions we use here. */
typedef enum nss_status (*set_function) (void);
@@ -20,17 +22,22 @@ compat_call (service_user *nip, const char *user, gid_t group, long int *start,
get_function getgrent_fct;
end_function endgrent_fct;
gid_t *groups = *groupsp;
+ __libc_lock_define_initialized(static, compat_lock);
getgrent_fct = __nss_lookup_function (nip, "getgrent_r");
if (getgrent_fct == NULL)
return NSS_STATUS_UNAVAIL;
setgrent_fct = __nss_lookup_function (nip, "setgrent");
+ __libc_lock_lock (compat_lock);
if (setgrent_fct)
{
status = DL_CALL_FCT (setgrent_fct, ());
if (status != NSS_STATUS_SUCCESS)
- return status;
+ {
+ __libc_lock_unlock (compat_lock);
+ return status;
+ }
}
endgrent_fct = __nss_lookup_function (nip, "endgrent");
@@ -122,5 +129,7 @@ compat_call (service_user *nip, const char *user, gid_t group, long int *start,
if (endgrent_fct)
DL_CALL_FCT (endgrent_fct, ());
+ __libc_lock_unlock (compat_lock);
+
return result;
}
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer