nss_winbind is not thread safe, any suggestions to fix this?
Jeremy Allison
jra at samba.org
Thu Sep 27 19:41:24 GMT 2007
On Thu, Sep 27, 2007 at 12:34:55PM -0700, Jeremy Allison wrote:
> On Fri, Sep 21, 2007 at 10:51:19PM +0800, boyang wrote:
> > Hi, all:
> >
> > Please review the patch, for 3_2_0
> >
> > thanks!
>
> Ok, I think this is a more complete (and correct) patch.
>
> Thanks for your initial work on this ! Much appreciated.
>
> If you could review this I'd greatly appreciate it.
And here's a version that actually compiles :-).
Thanks,
Jeremy
-------------- next part --------------
Index: Makefile.in
===================================================================
--- Makefile.in (revision 25374)
+++ Makefile.in (working copy)
@@ -1489,7 +1489,7 @@
@WINBIND_NSS@: $(BINARY_PREREQS) $(WINBIND_NSS_OBJ)
@echo "Linking $@"
@$(SHLD) $(WINBIND_NSS_LDSHFLAGS) -o $@ $(WINBIND_NSS_OBJ) \
- @WINBIND_NSS_EXTRA_LIBS@ @SONAMEFLAG@`basename $@`@NSSSONAMEVERSIONSUFFIX@
+ @WINBIND_NSS_EXTRA_LIBS@ @WINBIND_NSS_PTHREAD@ @SONAMEFLAG@`basename $@`@NSSSONAMEVERSIONSUFFIX@
@WINBIND_WINS_NSS@: $(BINARY_PREREQS) $(WINBIND_WINS_NSS_OBJ)
@echo "Linking $@"
Index: nsswitch/winbind_nss_linux.c
===================================================================
--- nsswitch/winbind_nss_linux.c (revision 25374)
+++ nsswitch/winbind_nss_linux.c (working copy)
@@ -21,6 +21,14 @@
#include "winbind_client.h"
+#if HAVE_PTHREAD_H
+#include <pthread.h>
+#endif
+
+#if HAVE_PTHREAD
+static pthread_mutex_t winbind_nss_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
/* Maximum number of users to pass back over the unix domain socket
per call. This is not a static limit on the total number of users
or groups returned in total. */
@@ -332,6 +340,10 @@
fprintf(stderr, "[%5d]: setpwent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
if (num_pw_cache > 0) {
ndx_pw_cache = num_pw_cache = 0;
winbindd_free_response(&getpwent_response);
@@ -342,6 +354,10 @@
fprintf(stderr, "[%5d]: setpwent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
return ret;
}
@@ -355,6 +371,10 @@
fprintf(stderr, "[%5d]: endpwent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
if (num_pw_cache > 0) {
ndx_pw_cache = num_pw_cache = 0;
winbindd_free_response(&getpwent_response);
@@ -365,6 +385,11 @@
fprintf(stderr, "[%5d]: endpwent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -382,6 +407,10 @@
fprintf(stderr, "[%5d]: getpwent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* Return an entry from the cache if we have one, or if we are
called again because we exceeded our static buffer. */
@@ -452,6 +481,10 @@
fprintf(stderr, "[%5d]: getpwent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
return ret;
}
@@ -464,14 +497,18 @@
NSS_STATUS ret;
static struct winbindd_response response;
struct winbindd_request request;
- static int keep_response=0;
+ static int keep_response_getpwuid=0;
#ifdef DEBUG_NSS
- fprintf(stderr, "[%5d]: getpwuid %d\n", getpid(), (unsigned int)uid);
+ fprintf(stderr, "[%5d]: getpwuid_r %d\n", getpid(), (unsigned int)uid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* If our static buffer needs to be expanded we are called again */
- if (!keep_response) {
+ if (!keep_response_getpwuid || uid != response.data.pw.pw_uid) {
/* Call for the first time */
@@ -487,7 +524,7 @@
&buffer, &buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getpwuid = true;
*errnop = errno = ERANGE;
goto done;
}
@@ -500,22 +537,27 @@
ret = fill_pwent(result, &response.data.pw, &buffer, &buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
*errnop = errno = ERANGE;
goto done;
}
- keep_response = false;
+ keep_response_getpwuid = false;
*errnop = errno = 0;
}
winbindd_free_response(&response);
+
done:
#ifdef DEBUG_NSS
fprintf(stderr, "[%5d]: getpwuid %d returns %s (%d)\n", getpid(),
(unsigned int)uid, nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -527,15 +569,19 @@
NSS_STATUS ret;
static struct winbindd_response response;
struct winbindd_request request;
- static int keep_response;
+ static int keep_response_getpwnam;
#ifdef DEBUG_NSS
- fprintf(stderr, "[%5d]: getpwnam %s\n", getpid(), name);
+ fprintf(stderr, "[%5d]: getpwnam_r %s\n", getpid(), name);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* If our static buffer needs to be expanded we are called again */
- if (!keep_response) {
+ if (!keep_response_getpwnam || strcmp(name,response.data.pw.pw_name) != 0) {
/* Call for the first time */
@@ -554,7 +600,7 @@
&buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getpwnam = true;
*errnop = errno = ERANGE;
goto done;
}
@@ -567,12 +613,12 @@
ret = fill_pwent(result, &response.data.pw, &buffer, &buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getpwnam = true;
*errnop = errno = ERANGE;
goto done;
}
- keep_response = false;
+ keep_response_getpwnam = false;
*errnop = errno = 0;
}
@@ -582,6 +628,11 @@
fprintf(stderr, "[%5d]: getpwnam %s returns %s (%d)\n", getpid(),
name, nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -604,6 +655,10 @@
fprintf(stderr, "[%5d]: setgrent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
if (num_gr_cache > 0) {
ndx_gr_cache = num_gr_cache = 0;
winbindd_free_response(&getgrent_response);
@@ -614,6 +669,11 @@
fprintf(stderr, "[%5d]: setgrent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -627,6 +687,10 @@
fprintf(stderr, "[%5d]: endgrent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
if (num_gr_cache > 0) {
ndx_gr_cache = num_gr_cache = 0;
winbindd_free_response(&getgrent_response);
@@ -637,6 +701,11 @@
fprintf(stderr, "[%5d]: endgrent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -656,6 +725,10 @@
fprintf(stderr, "[%5d]: getgrent\n", getpid());
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* Return an entry from the cache if we have one, or if we are
called again because we exceeded our static buffer. */
@@ -735,6 +808,11 @@
fprintf(stderr, "[%5d]: getgrent returns %s (%d)\n", getpid(),
nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -763,16 +841,21 @@
NSS_STATUS ret;
static struct winbindd_response response;
struct winbindd_request request;
- static int keep_response;
+ static int keep_response_getgrnam;
#ifdef DEBUG_NSS
fprintf(stderr, "[%5d]: getgrnam %s\n", getpid(), name);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* If our static buffer needs to be expanded we are called again */
-
- if (!keep_response) {
+ /* Or if the stored response group name differs from the request. */
+ if (!keep_response_getgrnam || strcmp(name,response.data.gr.gr_name) != 0) {
+
/* Call for the first time */
ZERO_STRUCT(request);
@@ -791,27 +874,27 @@
&buffer, &buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getgrnam = true;
*errnop = errno = ERANGE;
goto done;
}
}
} else {
-
+
/* We've been called again */
-
+
ret = fill_grent(result, &response.data.gr,
(char *)response.extra_data.data, &buffer,
&buflen);
-
+
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getgrnam = true;
*errnop = errno = ERANGE;
goto done;
}
- keep_response = false;
+ keep_response_getgrnam = false;
*errnop = 0;
}
@@ -821,6 +904,11 @@
fprintf(stderr, "[%5d]: getgrnam %s returns %s (%d)\n", getpid(),
name, nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -834,15 +922,20 @@
NSS_STATUS ret;
static struct winbindd_response response;
struct winbindd_request request;
- static int keep_response;
+ static int keep_response_getgrgid;
#ifdef DEBUG_NSS
fprintf(stderr, "[%5d]: getgrgid %d\n", getpid(), gid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
/* If our static buffer needs to be expanded we are called again */
+ /* Or if the stored response group name differs from the request. */
- if (!keep_response) {
+ if (!keep_response_getgrgid || gid != response.data.gr.gr_gid) {
/* Call for the first time */
@@ -860,7 +953,7 @@
&buffer, &buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getgrgid = true;
*errnop = errno = ERANGE;
goto done;
}
@@ -875,12 +968,12 @@
&buflen);
if (ret == NSS_STATUS_TRYAGAIN) {
- keep_response = true;
+ keep_response_getgrgid = true;
*errnop = errno = ERANGE;
goto done;
}
- keep_response = false;
+ keep_response_getgrgid = false;
*errnop = 0;
}
@@ -890,6 +983,10 @@
fprintf(stderr, "[%5d]: getgrgid %d returns %s (%d)\n", getpid(),
(unsigned int)gid, nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
return ret;
}
@@ -910,6 +1007,10 @@
user, group);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(request);
ZERO_STRUCT(response);
@@ -990,6 +1091,11 @@
fprintf(stderr, "[%5d]: initgroups %s returns %s (%d)\n", getpid(),
user, nss_err_str(ret), ret);
#endif
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1008,6 +1114,10 @@
fprintf(stderr, "[%5d]: getusersids %s\n", getpid(), user_sid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(request);
ZERO_STRUCT(response);
@@ -1033,6 +1143,11 @@
done:
winbindd_free_response(&response);
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1050,6 +1165,10 @@
fprintf(stderr, "[%5d]: nametosid %s\n", getpid(), name);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(response);
ZERO_STRUCT(request);
@@ -1075,6 +1194,11 @@
failed:
winbindd_free_response(&response);
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1093,6 +1217,10 @@
fprintf(stderr, "[%5d]: sidtoname %s\n", getpid(), sid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(response);
ZERO_STRUCT(request);
@@ -1139,6 +1267,11 @@
failed:
winbindd_free_response(&response);
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1154,6 +1287,10 @@
fprintf(stderr, "[%5d]: sidtouid %s\n", getpid(), sid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(request);
ZERO_STRUCT(response);
@@ -1169,6 +1306,11 @@
*uid = response.data.uid;
failed:
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1184,6 +1326,10 @@
fprintf(stderr, "[%5d]: sidtogid %s\n", getpid(), sid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(request);
ZERO_STRUCT(response);
@@ -1199,6 +1345,11 @@
*gid = response.data.gid;
failed:
+
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1215,6 +1366,10 @@
fprintf(stderr, "[%5u]: uidtosid %u\n", (unsigned int)getpid(), (unsigned int)uid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(response);
ZERO_STRUCT(request);
@@ -1238,6 +1393,11 @@
failed:
winbindd_free_response(&response);
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
@@ -1254,6 +1414,10 @@
fprintf(stderr, "[%5u]: gidtosid %u\n", (unsigned int)getpid(), (unsigned int)gid);
#endif
+#if HAVE_PTHREAD
+ pthread_mutex_lock(&winbind_nss_mutex);
+#endif
+
ZERO_STRUCT(response);
ZERO_STRUCT(request);
@@ -1277,5 +1441,10 @@
failed:
winbindd_free_response(&response);
+
+#if HAVE_PTHREAD
+ pthread_mutex_unlock(&winbind_nss_mutex);
+#endif
+
return ret;
}
Index: configure.in
===================================================================
--- configure.in (revision 25374)
+++ configure.in (working copy)
@@ -991,7 +991,7 @@
AC_CHECK_HEADERS(aio.h arpa/inet.h sys/fcntl.h sys/select.h fcntl.h sys/time.h sys/unistd.h rpc/nettype.h)
AC_CHECK_HEADERS(unistd.h utime.h grp.h sys/id.h memory.h alloca.h)
-AC_CHECK_HEADERS(limits.h float.h)
+AC_CHECK_HEADERS(limits.h float.h pthread.h)
AC_CHECK_HEADERS(rpc/rpc.h rpcsvc/nis.h rpcsvc/ypclnt.h)
AC_CHECK_HEADERS(sys/param.h ctype.h sys/wait.h sys/resource.h sys/ioctl.h sys/ipc.h sys/prctl.h)
AC_CHECK_HEADERS(sys/mman.h sys/filio.h sys/priv.h sys/shm.h string.h strings.h stdlib.h sys/socket.h)
@@ -5984,6 +5984,7 @@
WINBIND_WINS_NSS="nsswitch/libnss_wins.$SHLIBEXT"
WINBIND_NSS_LDSHFLAGS=$LDSHFLAGS
NSSSONAMEVERSIONSUFFIX=""
+WINBIND_NSS_PTHREAD=""
case "$host_os" in
linux*-gnu* | gnu* | k*bsd*-gnu)
@@ -6047,6 +6048,9 @@
;;
esac
+AC_CHECK_LIB(pthread, pthread_mutex_lock, [WINBIND_NSS_PTHREAD="-lpthread"
+ AC_DEFINE(HAVE_PTHREAD, 1, [whether pthread exists])])
+AC_SUBST(WINBIND_NSS_PTHREAD)
AC_SUBST(WINBIND_NSS)
AC_SUBST(WINBIND_WINS_NSS)
AC_SUBST(WINBIND_NSS_LDSHFLAGS)
More information about the samba-technical
mailing list