The branch, master has been updated
via 316b8fa4a8a nsswitch: remove winbind_nss_mutex
via 642a4452ce5 nsswitch: leverage TLS if available in favour over
global locking
via ae4a06f4b08 nsswitch: prepare for removing global locking by using
TLS
via 347f75499e8 nsswitch/stress-nss-libwbclient: also test after fork
via 29a99e5e123 libreplace: require TLS support if pthread support is
available
via 73e7d3731d8 libreplace: update comment on __thread support
from 9636b40b05b smbd: Use get_dirent_ea_size() also for
BOTH_DIRECTORY_INFO
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 316b8fa4a8ae1f5e48692c2a86c6c1c962953389
Author: Ralph Boehme <s...@samba.org>
Date: Wed Dec 21 14:48:06 2022 +0100
nsswitch: remove winbind_nss_mutex
We're now thread-safe by using TLS, so the global lock isn't needed anymore.
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
Autobuild-User(master): Ralph Böhme <s...@samba.org>
Autobuild-Date(master): Thu Jan 5 12:34:35 UTC 2023 on sn-devel-184
commit 642a4452ce5b3333c50e41e54bc6ca779686ecc3
Author: Ralph Boehme <s...@samba.org>
Date: Sun Nov 6 16:57:27 2022 +0100
nsswitch: leverage TLS if available in favour over global locking
The global locking can lead to deadlocks when using nscd: when processing
the
first request in winbind, when we know we call into code that will recurse
into
winbind we call winbind_off() which sets an environment variable which is
later
checked here in the nsswitch module.
But with nscd in the stack, we don't see the env variable in nsswitch, so
when
we try to acquire the global lock again, it is already locked and we
deadlock.
By using a thread specific winbindd_context, plus a few other thread local
global
variables, we don't need a global lock anymore.
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit ae4a06f4b087c6b247f55716a4b3f59aaa333379
Author: Ralph Boehme <s...@samba.org>
Date: Sun Nov 6 16:57:27 2022 +0100
nsswitch: prepare for removing global locking by using TLS
Switch to using TLS for all global variables. No change in behaviour.
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 347f75499e832dc669268c5c1b0368224dbf0374
Author: Ralph Boehme <s...@samba.org>
Date: Mon Oct 31 16:19:21 2022 +0100
nsswitch/stress-nss-libwbclient: also test after fork
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 29a99e5e123465145f0faf66bddd94ecc26d15ff
Author: Ralph Boehme <s...@samba.org>
Date: Tue Nov 15 11:30:28 2022 +0100
libreplace: require TLS support if pthread support is available
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 73e7d3731d87b3c3ed907e718fcba5ed2e293e51
Author: Ralph Boehme <s...@samba.org>
Date: Thu Oct 27 07:51:49 2022 +0200
libreplace: update comment on __thread support
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
lib/replace/replace.h | 12 +++
lib/replace/wscript | 6 +-
nsswitch/libwbclient/wscript | 6 +-
nsswitch/stress-nss-libwbclient.c | 152 ++++++++++++++++++++++++++++++++++++++
nsswitch/wb_common.c | 127 +++++++++++++++++++++++++------
nsswitch/winbind_nss_linux.c | 123 +++++-------------------------
6 files changed, 296 insertions(+), 130 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/replace/replace.h b/lib/replace/replace.h
index de50761d000..b15f3d14c8a 100644
--- a/lib/replace/replace.h
+++ b/lib/replace/replace.h
@@ -1082,4 +1082,16 @@ static inline bool hex_byte(const char *in, uint8_t *out)
#include <sys/atomic.h>
#endif
+/*
+ * This handles the case of missing pthread support and ensures code can use
+ * __thread unconditionally, such that when built on a platform without pthread
+ * support, the __thread qualifier is an empty define.
+ */
+#ifndef HAVE___THREAD
+# ifdef HAVE_PTHREAD
+# error Configure failed to detect pthread library with missing TLS support
+# endif
+#define HAVE___THREAD
+#endif
+
#endif /* _LIBREPLACE_REPLACE_H */
diff --git a/lib/replace/wscript b/lib/replace/wscript
index b1ca95515a0..82c5a8a477b 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -673,7 +673,8 @@ syscall(SYS_copy_file_range,0,NULL,0,NULL,0,0);
conf.CONFIG_SET('HAVE_PTHREAD_MUTEX_CONSISTENT_NP'))):
conf.DEFINE('HAVE_ROBUST_MUTEXES', 1)
- # __thread is available since 2002 in gcc.
+ # __thread is available in Solaris Studio, IBM XL,
+ # gcc, Clang and Intel C Compiler
conf.CHECK_CODE('''
__thread int tls;
@@ -685,6 +686,9 @@ syscall(SYS_copy_file_range,0,NULL,0,NULL,0,0);
addmain=False,
msg='Checking for __thread local storage')
+ if conf.CONFIG_SET('HAVE_PTHREAD') and not
conf.CONFIG_SET('HAVE___THREAD'):
+ conf.fatal('Missing required TLS support in pthread library')
+
conf.CHECK_FUNCS_IN('crypt', 'crypt', checklibc=True)
conf.CHECK_FUNCS_IN('crypt_r', 'crypt', checklibc=True)
conf.CHECK_FUNCS_IN('crypt_rn', 'crypt', checklibc=True)
diff --git a/nsswitch/libwbclient/wscript b/nsswitch/libwbclient/wscript
index 51c662bac45..e81cd92abc8 100644
--- a/nsswitch/libwbclient/wscript
+++ b/nsswitch/libwbclient/wscript
@@ -27,9 +27,13 @@ def build(bld):
#
# Logs.info("\tSelected embedded libwbclient build")
+ wbclient_internal_deps = 'replace'
+ if bld.CONFIG_SET('HAVE_PTHREAD'):
+ wbclient_internal_deps += ' pthread'
+
bld.SAMBA_SUBSYSTEM('wbclient-internal',
source='../wb_common.c',
- deps='replace',
+ deps=wbclient_internal_deps,
cflags='-DWINBINDD_SOCKET_DIR=\"%s\"' % bld.env.WINBINDD_SOCKET_DIR,
hide_symbols=True,
provide_builtin_linking=True,
diff --git a/nsswitch/stress-nss-libwbclient.c
b/nsswitch/stress-nss-libwbclient.c
index df1d85c9c40..35818530886 100644
--- a/nsswitch/stress-nss-libwbclient.c
+++ b/nsswitch/stress-nss-libwbclient.c
@@ -30,6 +30,9 @@
#include <sys/types.h>
#include <pwd.h>
#include <wbclient.h>
+#include <sys/socket.h>
+#include <errno.h>
+#include <assert.h>
#define RUNTIME 10
@@ -46,8 +49,11 @@ static void *query_nss_thread(void *ptr)
{
struct thread_state *state = ptr;
char buf[1024];
+ ssize_t nread, nwritten;
+ int p[2];
int rc;
struct passwd pwd, *result;
+ pid_t pid;
while (time(NULL) < state->timeout) {
rc = getpwnam_r(state->username,
@@ -73,6 +79,82 @@ static void *query_nss_thread(void *ptr)
}
pthread_mutex_unlock(&state->lock);
}
+
+ rc = socketpair(AF_UNIX, SOCK_STREAM, 0, p);
+ if (rc != 0) {
+ state->fail = true;
+ return NULL;
+ }
+
+ /*
+ * Check getpwnam_r() still works after a fork,
+ * both in parent and child.
+ */
+
+ pid = fork();
+ if (pid == -1) {
+ return NULL;
+ }
+ if (pid == 0) {
+ /* Child */
+ rc = getpwnam_r(state->username,
+ &pwd,
+ buf,
+ sizeof(buf),
+ &result);
+ if (rc != 0 || result == NULL) {
+ fprintf(stderr,
+ "getpwnam_r failed with rc='%s' result=%p\n",
+ strerror(rc),
+ result);
+ rc = 1;
+ nwritten = write(p[0], &rc, sizeof(int));
+ assert(nwritten == sizeof(int));
+ exit(1);
+ }
+ printf("child: getpwnam_r in child succeeded\n");
+ rc = 0;
+ nwritten = write(p[0], &rc, sizeof(int));
+ assert(nwritten == sizeof(int));
+ exit(1);
+ }
+
+ /* Parent */
+
+ /* Check result from child */
+ nread = read(p[1], &rc, sizeof(int));
+ if (nread != sizeof(int)) {
+ fprintf(stderr,
+ "read from child failed with errno='%s' nread=%zd\n",
+ strerror(errno),
+ nread);
+ state->fail = true;
+ return NULL;
+ }
+
+ if (rc != 0) {
+ fprintf(stderr,
+ "getpwnam_r failed in the child\n");
+ state->fail = true;
+ return NULL;
+ }
+ printf("parent: getpwnam_r in child succeeded\n");
+
+ /* Verify getpwnam_r() in parent after fork */
+ rc = getpwnam_r(state->username,
+ &pwd,
+ buf,
+ sizeof(buf),
+ &result);
+ if (rc != 0 || result == NULL) {
+ fprintf(stderr,
+ "getpwnam_r failed with rc='%s' result=%p\n",
+ strerror(rc),
+ result);
+ state->fail = true;
+ return NULL;
+ }
+ printf("parent: getpwnam_r in parent succeeded\n");
return NULL;
}
@@ -81,6 +163,10 @@ static void *query_wbc_thread(void *ptr)
struct thread_state *state = ptr;
struct passwd *ppwd;
wbcErr wbc_status;
+ pid_t pid;
+ ssize_t nread, nwritten;
+ int p[2];
+ int rc;
while (time(NULL) < state->timeout) {
wbc_status = wbcGetpwnam(state->username, &ppwd);
@@ -102,6 +188,72 @@ static void *query_wbc_thread(void *ptr)
}
pthread_mutex_unlock(&state->lock);
}
+
+ rc = socketpair(AF_UNIX, SOCK_STREAM, 0, p);
+ if (rc != 0) {
+ state->fail = true;
+ return NULL;
+ }
+
+ /*
+ * Check wbcGetpwnam() still works after a fork,
+ * both in parent and child.
+ */
+
+ pid = fork();
+ if (pid == -1) {
+ return NULL;
+ }
+ if (pid == 0) {
+ /* Child */
+ wbc_status = wbcGetpwnam(state->username, &ppwd);
+ if (!WBC_ERROR_IS_OK(wbc_status)) {
+ fprintf(stderr,
+ "wbcGetpwnam failed with %s\n",
+ wbcErrorString(wbc_status));
+ rc = 1;
+ nwritten = write(p[0], &rc, sizeof(int));
+ assert(nwritten == sizeof(int));
+ exit(1);
+ }
+ printf("child: wbcGetpwnam in child succeeded\n");
+ rc = 0;
+ nwritten = write(p[0], &rc, sizeof(int));
+ assert(nwritten == sizeof(int));
+ exit(1);
+ }
+
+ /* Parent */
+
+ /* Check result from child */
+ nread = read(p[1], &rc, sizeof(int));
+ if (nread != sizeof(int)) {
+ fprintf(stderr,
+ "read from child failed with errno='%s' nread=%zd\n",
+ strerror(errno),
+ nread);
+ state->fail = true;
+ return NULL;
+ }
+
+ if (rc != 0) {
+ fprintf(stderr,
+ "wbcGetpwnam failed in the child\n");
+ state->fail = true;
+ return NULL;
+ }
+ printf("parent: wbcGetpwnam in child succeeded\n");
+
+ /* Verify wbcGetpwnam() in parent after fork */
+ wbc_status = wbcGetpwnam(state->username, &ppwd);
+ if (!WBC_ERROR_IS_OK(wbc_status)) {
+ fprintf(stderr,
+ "wbcGetpwnam failed with %s\n",
+ wbcErrorString(wbc_status));
+ state->fail = true;
+ return NULL;
+ }
+ printf("parent: wbcGetpwnam in parent succeeded\n");
return NULL;
}
diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 1a3ed1241c5..7ae3a11162d 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -26,12 +26,13 @@
#include "replace.h"
#include "system/select.h"
#include "winbind_client.h"
+#include <assert.h>
#ifdef HAVE_PTHREAD_H
#include <pthread.h>
#endif
-static char client_name[32];
+static __thread char client_name[32];
/* Global context */
@@ -41,30 +42,115 @@ struct winbindd_context {
pid_t our_pid; /* calling process pid */
};
+static struct wb_global_ctx {
#ifdef HAVE_PTHREAD
-static pthread_mutex_t wb_global_ctx_mutex = PTHREAD_MUTEX_INITIALIZER;
+ pthread_once_t control;
+ pthread_key_t key;
+#else
+ bool dummy;
+#endif
+} wb_global_ctx = {
+#ifdef HAVE_PTHREAD
+ .control = PTHREAD_ONCE_INIT,
#endif
+};
-static struct winbindd_context *get_wb_global_ctx(void)
+static void winbind_close_sock(struct winbindd_context *ctx);
+
+#ifdef HAVE_PTHREAD
+static void wb_thread_ctx_initialize(void);
+
+static void wb_atfork_child(void)
+{
+ struct winbindd_context *ctx = NULL;
+ int ret;
+
+ ctx = (struct winbindd_context *)pthread_getspecific(wb_global_ctx.key);
+ if (ctx == NULL) {
+ return;
+ }
+
+ ret = pthread_setspecific(wb_global_ctx.key, NULL);
+ assert(ret == 0);
+
+ winbind_close_sock(ctx);
+ free(ctx);
+
+ ret = pthread_key_delete(wb_global_ctx.key);
+ assert(ret == 0);
+
+ wb_global_ctx.control = (pthread_once_t)PTHREAD_ONCE_INIT;
+}
+
+static void wb_thread_ctx_destructor(void *p)
+{
+ struct winbindd_context *ctx = (struct winbindd_context *)p;
+
+ winbind_close_sock(ctx);
+ free(ctx);
+}
+
+static void wb_thread_ctx_initialize(void)
+{
+ int ret;
+
+ ret = pthread_atfork(NULL,
+ NULL,
+ wb_atfork_child);
+ assert(ret == 0);
+
+ ret = pthread_key_create(&wb_global_ctx.key,
+ wb_thread_ctx_destructor);
+ assert(ret == 0);
+}
+#endif
+
+static struct winbindd_context *get_wb_thread_ctx(void)
{
- static struct winbindd_context wb_global_ctx = {
+ struct winbindd_context *ctx = NULL;
+ int ret;
+
+ ret = pthread_once(&wb_global_ctx.control,
+ wb_thread_ctx_initialize);
+ assert(ret == 0);
+
+ ctx = (struct winbindd_context *)pthread_getspecific(
+ wb_global_ctx.key);
+ if (ctx != NULL) {
+ return ctx;
+ }
+
+ ctx = malloc(sizeof(struct winbindd_context));
+ if (ctx == NULL) {
+ return NULL;
+ }
+
+ *ctx = (struct winbindd_context) {
.winbindd_fd = -1,
.is_privileged = false,
.our_pid = 0
};
-#ifdef HAVE_PTHREAD
- pthread_mutex_lock(&wb_global_ctx_mutex);
-#endif
- return &wb_global_ctx;
+ ret = pthread_setspecific(wb_global_ctx.key, ctx);
+ if (ret != 0) {
+ free(ctx);
+ return NULL;
+ }
+ return ctx;
}
-static void put_wb_global_ctx(void)
+static struct winbindd_context *get_wb_global_ctx(void)
{
#ifdef HAVE_PTHREAD
- pthread_mutex_unlock(&wb_global_ctx_mutex);
+ return get_wb_thread_ctx();
+#else
+ static struct winbindd_context ctx = {
+ .winbindd_fd = -1,
+ .is_privileged = false,
+ .our_pid = 0
+ };
+ return &ctx;
#endif
- return;
}
void winbind_set_client_name(const char *name)
@@ -148,9 +234,16 @@ static void winbind_destructor(void)
{
struct winbindd_context *ctx;
+#ifdef HAVE_PTHREAD_H
+ ctx = (struct winbindd_context *)pthread_getspecific(wb_global_ctx.key);
+ if (ctx == NULL) {
+ return;
+ }
+#else
ctx = get_wb_global_ctx();
+#endif
+
winbind_close_sock(ctx);
- put_wb_global_ctx();
}
#define CONNECT_TIMEOUT 30
@@ -782,11 +875,9 @@ NSS_STATUS winbindd_request_response(struct
winbindd_context *ctx,
struct winbindd_response *response)
{
NSS_STATUS status = NSS_STATUS_UNAVAIL;
- bool release_global_ctx = false;
if (ctx == NULL) {
ctx = get_wb_global_ctx();
- release_global_ctx = true;
}
status = winbindd_send_request(ctx, req_type, 0, request);
@@ -796,9 +887,6 @@ NSS_STATUS winbindd_request_response(struct
winbindd_context *ctx,
status = winbindd_get_response(ctx, response);
out:
- if (release_global_ctx) {
- put_wb_global_ctx();
- }
return status;
}
@@ -808,11 +896,9 @@ NSS_STATUS winbindd_priv_request_response(struct
winbindd_context *ctx,
struct winbindd_response *response)
{
NSS_STATUS status = NSS_STATUS_UNAVAIL;
- bool release_global_ctx = false;
if (ctx == NULL) {
ctx = get_wb_global_ctx();
- release_global_ctx = true;
}
status = winbindd_send_request(ctx, req_type, 1, request);
@@ -822,9 +908,6 @@ NSS_STATUS winbindd_priv_request_response(struct
winbindd_context *ctx,
status = winbindd_get_response(ctx, response);
out:
- if (release_global_ctx) {
- put_wb_global_ctx();
- }
return status;
}
diff --git a/nsswitch/winbind_nss_linux.c b/nsswitch/winbind_nss_linux.c
index a19c86dcdcc..4694f25d7ed 100644
--- a/nsswitch/winbind_nss_linux.c
+++ b/nsswitch/winbind_nss_linux.c
@@ -25,10 +25,6 @@
#include <pthread.h>
#endif
-#ifdef 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. */
@@ -354,10 +350,10 @@ static NSS_STATUS fill_grent(struct group *result, struct
winbindd_gr *gr,
* NSS user functions
*/
-static struct winbindd_response getpwent_response;
+static __thread struct winbindd_response getpwent_response;
-static int ndx_pw_cache; /* Current index into pwd cache */
-static int num_pw_cache; /* Current size of pwd cache */
+static __thread int ndx_pw_cache; /* Current index into pwd cache */
+static __thread int num_pw_cache; /* Current size of pwd cache */
/* Rewind "file pointer" to start of ntdom password database */
@@ -370,10 +366,6 @@ _nss_winbind_setpwent(void)
fprintf(stderr, "[%5d]: setpwent\n", getpid());
#endif
-#ifdef HAVE_PTHREAD
- pthread_mutex_lock(&winbind_nss_mutex);
-#endif
--
Samba Shared Repository
