[PATCH] make nsswitch and libwbclient thread safe

Ralph Wuerthner ralphw at de.ibm.com
Thu Oct 4 14:36:25 UTC 2018


Hi!

Please see attached patchset to make nsswitch & libwbclient thread safe.

Currently nsswitch & libwbclient share a common, global context when 
communicating to winbindd. Threaded applications submitting requests in 
parallel via nsswitch & libwbclient to winbindd can fail because of the 
simplistic design of the communication protocol.

With the patch applied, access to the global libwbclient context is 
serialized by a mutex, similar to the mutex used in winbind_nss_linux.c.

Regards

    Ralph Wuerthner
-------------- next part --------------
From e3fab2c05e3570de1df6f583dbc7612eb1f39a84 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Tue, 2 Oct 2018 10:58:12 +0200
Subject: [PATCH 1/3] nsswitch: use goto to have only one function return

Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
---
 nsswitch/wb_common.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 42b341b..c2f67d7 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -731,10 +731,12 @@ NSS_STATUS winbindd_request_response(struct winbindd_context *ctx,
 	}
 
 	status = winbindd_send_request(ctx, req_type, 0, request);
-	if (status != NSS_STATUS_SUCCESS)
-		return (status);
+	if (status != NSS_STATUS_SUCCESS) {
+		goto out;
+	}
 	status = winbindd_get_response(ctx, response);
 
+out:
 	return status;
 }
 
@@ -750,10 +752,12 @@ NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx,
 	}
 
 	status = winbindd_send_request(ctx, req_type, 1, request);
-	if (status != NSS_STATUS_SUCCESS)
-		return (status);
+	if (status != NSS_STATUS_SUCCESS) {
+		goto out;
+	}
 	status = winbindd_get_response(ctx, response);
 
+out:
 	return status;
 }
 
-- 
2.7.4


From 2e65569787c32d5ff0397492474b06f0b5423ddb Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Tue, 2 Oct 2018 13:35:16 +0200
Subject: [PATCH 2/3] nsswitch: make wb_global_ctx private add add get/put
 functions to access global context

Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
---
 nsswitch/wb_common.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index c2f67d7..9ba74c3 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -35,11 +35,22 @@ struct winbindd_context {
 	pid_t our_pid;		/* calling process pid */
 };
 
-static struct winbindd_context wb_global_ctx = {
-	.winbindd_fd = -1,
-	.is_privileged = false,
-	.our_pid = 0
-};
+static struct winbindd_context *get_wb_global_ctx(void)
+{
+	static struct winbindd_context wb_global_ctx = {
+		.winbindd_fd = -1,
+		.is_privileged = false,
+		.our_pid = 0
+	};
+
+	return &wb_global_ctx;
+}
+
+static void put_wb_global_ctx(void)
+{
+	/* noop for now */
+	return;
+}
 
 /* Free a response structure */
 
@@ -93,7 +104,11 @@ __attribute__((destructor))
 #endif
 static void winbind_destructor(void)
 {
-	winbind_close_sock(&wb_global_ctx);
+	struct winbindd_context *ctx;
+
+	ctx = get_wb_global_ctx();
+	winbind_close_sock(ctx);
+	put_wb_global_ctx();
 }
 
 #define CONNECT_TIMEOUT 30
@@ -725,9 +740,11 @@ 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 = &wb_global_ctx;
+		ctx = get_wb_global_ctx();
+		release_global_ctx = true;
 	}
 
 	status = winbindd_send_request(ctx, req_type, 0, request);
@@ -737,6 +754,9 @@ 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;
 }
 
@@ -746,9 +766,11 @@ 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 = &wb_global_ctx;
+		ctx = get_wb_global_ctx();
+		release_global_ctx = true;
 	}
 
 	status = winbindd_send_request(ctx, req_type, 1, request);
@@ -758,6 +780,9 @@ 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;
 }
 
-- 
2.7.4


From 1fcb145f97352dcdc5e75b29706129222a442a25 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Tue, 2 Oct 2018 13:41:00 +0200
Subject: [PATCH 3/3] nsswitch: protect access to wb_global_ctx by a mutex

This change will make libwbclient thread safe for all API calls not using a
context. Especially there are no more conflicts with threads using nsswitch
and libwbclient in parallel.

Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
---
 nsswitch/wb_common.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 9ba74c3..59370aa 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -27,6 +27,10 @@
 #include "system/select.h"
 #include "winbind_client.h"
 
+#if HAVE_PTHREAD_H
+#include <pthread.h>
+#endif
+
 /* Global context */
 
 struct winbindd_context {
@@ -35,6 +39,10 @@ struct winbindd_context {
 	pid_t our_pid;		/* calling process pid */
 };
 
+#if HAVE_PTHREAD
+static pthread_mutex_t wb_global_ctx_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 static struct winbindd_context *get_wb_global_ctx(void)
 {
 	static struct winbindd_context wb_global_ctx = {
@@ -43,12 +51,17 @@ static struct winbindd_context *get_wb_global_ctx(void)
 		.our_pid = 0
 	};
 
+#if HAVE_PTHREAD
+	pthread_mutex_lock(&wb_global_ctx_mutex);
+#endif
 	return &wb_global_ctx;
 }
 
 static void put_wb_global_ctx(void)
 {
-	/* noop for now */
+#if HAVE_PTHREAD
+	pthread_mutex_unlock(&wb_global_ctx_mutex);
+#endif
 	return;
 }
 
-- 
2.7.4



More information about the samba-technical mailing list