[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu Nov 1 04:07:03 UTC 2018


The branch, master has been updated
       via  40bd0a9 nsswitch: Run nsswitch thread test
       via  b5ea794 nsswitch: add test for parallel NSS & libwbclient calls
       via  988182c nsswitch: protect access to wb_global_ctx by a mutex
       via  e82b3ac nsswitch: make wb_global_ctx private add add get/put functions to access global context
       via  2cfb58d nsswitch: use goto to have only one function return
       via  8d14714 s3: winbind: Remove fstring from wb_acct_info struct
      from  1b2de44 vfs_fruit: let fruit_open_meta() with O_CREAT return a fake-fd

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 40bd0a930b2990a6c4e10a5e0db27608cff198c8
Author: Volker Lendecke <vl at samba.org>
Date:   Fri Oct 5 16:27:48 2018 +0200

    nsswitch: Run nsswitch thread test
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu Nov  1 05:06:23 CET 2018 on sn-devel-144

commit b5ea7946f844ff6b9337fea57fd002b716595703
Author: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date:   Fri Oct 5 13:53:30 2018 +0200

    nsswitch: add test for parallel NSS & libwbclient calls
    
    Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 988182c3b8dab6511c75622e3484c9ef71e6d0db
Author: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date:   Tue Oct 2 13:41:00 2018 +0200

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit e82b3ac0ae5b50993db3c85d77dd317f957beb7e
Author: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date:   Tue Oct 2 13:35:16 2018 +0200

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 2cfb58d7535e63c5ef9f3970ebaa189741b715cb
Author: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date:   Tue Oct 2 10:58:12 2018 +0200

    nsswitch: use goto to have only one function return
    
    Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 8d14714cc5e4c2b439e973faf602afb2fc1e7488
Author: Samuel Cabrero <scabrero at suse.de>
Date:   Tue Oct 30 18:47:16 2018 +0100

    s3: winbind: Remove fstring from wb_acct_info struct
    
    The group enumeration backend functions try to allocate an array of
    wb_acct_info structs with a number of elements equal to the number of
    groups. In domains with a large number of groups this allocation may
    fail due to the size of the chunk.
    
    Found while trying to enumerate the groups in a domain with more than
    700k groups.
    
    Signed-off-by: Samuel Cabrero <scabrero at suse.de>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 nsswitch/stress-nss-libwbclient.c                | 156 +++++++++++++++++++++++
 nsswitch/wb_common.c                             |  66 ++++++++--
 nsswitch/wscript_build                           |   7 +
 source3/script/tests/test_libwbclient_threads.sh |  17 +++
 source3/selftest/tests.py                        |   7 +
 source3/winbindd/winbindd.h                      |   4 +-
 source3/winbindd/winbindd_ads.c                  |   8 +-
 source3/winbindd/winbindd_cache.c                |   8 +-
 source3/winbindd/winbindd_rpc.c                  |  16 ++-
 9 files changed, 263 insertions(+), 26 deletions(-)
 create mode 100644 nsswitch/stress-nss-libwbclient.c
 create mode 100755 source3/script/tests/test_libwbclient_threads.sh


Changeset truncated at 500 lines:

diff --git a/nsswitch/stress-nss-libwbclient.c b/nsswitch/stress-nss-libwbclient.c
new file mode 100644
index 0000000..cf85ff3
--- /dev/null
+++ b/nsswitch/stress-nss-libwbclient.c
@@ -0,0 +1,156 @@
+/*
+   Unix SMB/CIFS implementation.
+
+   Stress test for parallel NSS & libwbclient calls.
+
+   Copyright (C) Ralph Wuerthner 2018
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <pthread.h>
+#include <string.h>
+#include <unistd.h>
+#include <time.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <pwd.h>
+#include <wbclient.h>
+
+#define RUNTIME 10
+
+struct thread_state {
+	const char *username;
+	time_t timeout;
+	pthread_mutex_t lock;
+	bool fail;
+	int nss_loop_count;
+	int wbc_loop_count;
+};
+
+static void *query_nss_thread(void *ptr)
+{
+	struct thread_state *state = ptr;
+	char buf[1024];
+	int rc;
+	struct passwd pwd, *result;
+
+	while (time(NULL) < state->timeout) {
+		rc = getpwnam_r(state->username,
+				&pwd,
+				buf,
+				sizeof(buf),
+				&result);
+		if (rc != 0 || result == NULL) {
+			pthread_mutex_lock(&state->lock);
+			state->fail = true;
+			pthread_mutex_unlock(&state->lock);
+			fprintf(stderr,
+				"getpwnam_r failed with rc='%s' result=%p\n",
+				strerror(rc),
+				result);
+			break;
+		}
+		state->nss_loop_count++;
+		pthread_mutex_lock(&state->lock);
+		if (state->fail) {
+			pthread_mutex_unlock(&state->lock);
+			break;
+		}
+		pthread_mutex_unlock(&state->lock);
+	}
+	return NULL;
+}
+
+static void *query_wbc_thread(void *ptr)
+{
+	struct thread_state *state = ptr;
+	struct passwd *ppwd;
+	wbcErr wbc_status;
+
+	while (time(NULL) < state->timeout) {
+		wbc_status = wbcGetpwnam(state->username, &ppwd);
+		if (!WBC_ERROR_IS_OK(wbc_status)) {
+			pthread_mutex_lock(&state->lock);
+			state->fail = true;
+			pthread_mutex_unlock(&state->lock);
+			fprintf(stderr,
+				"wbcGetpwnam failed with %s\n",
+				wbcErrorString(wbc_status));
+			break;
+		}
+		wbcFreeMemory(ppwd);
+		state->wbc_loop_count++;
+		pthread_mutex_lock(&state->lock);
+		if (state->fail) {
+			pthread_mutex_unlock(&state->lock);
+			break;
+		}
+		pthread_mutex_unlock(&state->lock);
+	}
+	return NULL;
+}
+
+int main(int argc, char *argv[])
+{
+	int rc, n;
+	struct thread_state state;
+	pthread_t threads[2];
+
+	if (argc < 2 ) {
+		fprintf(stderr,"%s: missing domain user\n", argv[0]);
+		return 1;
+	}
+
+	state.username = argv[1];
+	state.timeout = time(NULL) + RUNTIME;
+	pthread_mutex_init(&state.lock, NULL);
+	state.fail = false;
+	state.nss_loop_count = 0;
+	state.wbc_loop_count = 0;
+
+	printf("query domain user '%s'\n", state.username);
+
+	/* create query threads */
+	rc = pthread_create(&threads[0], NULL, query_nss_thread, &state);
+	if (rc != 0) {
+		fprintf(stderr,
+			"creating NSS thread failed: %s\n",
+			strerror(rc));
+		exit(1);
+	}
+	rc = pthread_create(&threads[1], NULL, query_wbc_thread, &state);
+	if (rc != 0) {
+		fprintf(stderr,
+			"creating libwbclient thread failed: %s\n",
+			strerror(rc));
+		exit(1);
+	}
+
+	/* wait for query threads to terminate */
+	for (n = 0; n < 2; n++) {
+		pthread_join(threads[n], NULL);
+	}
+
+	fprintf(state.fail ? stderr: stdout,
+		"test %s with %i NSS and %i libwbclient calls\n",
+		state.fail ? "failed" : "passed",
+		state.nss_loop_count,
+		state.wbc_loop_count);
+
+	return state.fail;
+}
diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 42b341b..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,11 +39,31 @@ 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
-};
+#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 = {
+		.winbindd_fd = -1,
+		.is_privileged = false,
+		.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)
+{
+#if HAVE_PTHREAD
+	pthread_mutex_unlock(&wb_global_ctx_mutex);
+#endif
+	return;
+}
 
 /* Free a response structure */
 
@@ -93,7 +117,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,16 +753,23 @@ 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);
-	if (status != NSS_STATUS_SUCCESS)
-		return (status);
+	if (status != NSS_STATUS_SUCCESS) {
+		goto out;
+	}
 	status = winbindd_get_response(ctx, response);
 
+out:
+	if (release_global_ctx) {
+		put_wb_global_ctx();
+	}
 	return status;
 }
 
@@ -744,16 +779,23 @@ 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);
-	if (status != NSS_STATUS_SUCCESS)
-		return (status);
+	if (status != NSS_STATUS_SUCCESS) {
+		goto out;
+	}
 	status = winbindd_get_response(ctx, response);
 
+out:
+	if (release_global_ctx) {
+		put_wb_global_ctx();
+	}
 	return status;
 }
 
diff --git a/nsswitch/wscript_build b/nsswitch/wscript_build
index ff98372..6acc4a1 100644
--- a/nsswitch/wscript_build
+++ b/nsswitch/wscript_build
@@ -17,6 +17,13 @@ bld.SAMBA_BINARY('nsstest',
                  install=False
 		 )
 
+if bld.CONFIG_SET('HAVE_PTHREAD'):
+    bld.SAMBA_BINARY('stress-nss-libwbclient',
+		     source='stress-nss-libwbclient.c',
+		     deps='wbclient',
+		     install=False
+		     )
+
 # The nss_wrapper code relies strictly on the linux implementation and
 # name, so compile but do not install a copy under this name.
 bld.SAMBA_LIBRARY('nss_wrapper_winbind',
diff --git a/source3/script/tests/test_libwbclient_threads.sh b/source3/script/tests/test_libwbclient_threads.sh
new file mode 100755
index 0000000..1f8d041
--- /dev/null
+++ b/source3/script/tests/test_libwbclient_threads.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+if [ $# -lt 2 ] ; then
+cat <<EOF
+Usage: test_libwbclient_threads.sh DOMAIN USERNAME
+EOF
+exit 1;
+fi
+
+DOMAIN="$1"
+USERNAME="$2"
+shift 2
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+testit "libwbclient-threads" "$BINDIR/stress-nss-libwbclient" "$DOMAIN/$USERNAME"
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index f31007f..08395c1 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -385,6 +385,13 @@ if with_pthreadpool and have_ldwrap:
     plantestsuite("samba3.pthreadpool_cmocka", "none",
                   [os.path.join(bindir(), "pthreadpooltest_cmocka")])
 
+if with_pthreadpool:
+    plantestsuite("samba3.libwbclient_threads",
+                  "nt4_member",
+                  [os.path.join(samba3srcdir,
+                                "script/tests/test_libwbclient_threads.sh"),
+                   "$DOMAIN", "$DC_USERNAME"])
+
 plantestsuite("samba3.async_req", "nt4_dc",
               [os.path.join(samba3srcdir, "script/tests/test_async_req.sh")])
 
diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 5737176..6d4b92f 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -189,8 +189,8 @@ struct winbindd_domain {
 };
 
 struct wb_acct_info {
-	fstring acct_name; /* account name */
-	fstring acct_desc; /* account name */
+	const char *acct_name; /* account name */
+	const char *acct_desc; /* account name */
 	uint32_t rid; /* domain-relative RID */
 };
 
diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index 76d6a30..abc044d 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -500,8 +500,8 @@ static NTSTATUS enum_dom_groups(struct winbindd_domain *domain,
 		struct dom_sid sid;
 		uint32_t rid;
 
-		name = ads_pull_username(ads, mem_ctx, msg);
-		gecos = ads_pull_string(ads, mem_ctx, msg, "name");
+		name = ads_pull_username(ads, (*info), msg);
+		gecos = ads_pull_string(ads, (*info), msg, "name");
 		if (!ads_pull_sid(ads, msg, "objectSid", &sid)) {
 			DEBUG(1,("No sid for %s !?\n", name));
 			continue;
@@ -512,8 +512,8 @@ static NTSTATUS enum_dom_groups(struct winbindd_domain *domain,
 			continue;
 		}
 
-		fstrcpy((*info)[i].acct_name, name);
-		fstrcpy((*info)[i].acct_desc, gecos);
+		(*info)[i].acct_name = name;
+		(*info)[i].acct_desc = gecos;
 		(*info)[i].rid = rid;
 		i++;
 	}
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 7d98d61..2e2a04d 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1565,8 +1565,8 @@ do_fetch_cache:
 		smb_panic_fn("enum_dom_groups out of memory");
 	}
 	for (i=0; i<(*num_entries); i++) {
-		fstrcpy((*info)[i].acct_name, centry_string(centry, mem_ctx));
-		fstrcpy((*info)[i].acct_desc, centry_string(centry, mem_ctx));
+		(*info)[i].acct_name = centry_string(centry, (*info));
+		(*info)[i].acct_desc = centry_string(centry, (*info));
 		(*info)[i].rid = centry_uint32(centry);
 	}
 
@@ -1660,8 +1660,8 @@ do_fetch_cache:
 		smb_panic_fn("enum_dom_groups out of memory");
 	}
 	for (i=0; i<(*num_entries); i++) {
-		fstrcpy((*info)[i].acct_name, centry_string(centry, mem_ctx));
-		fstrcpy((*info)[i].acct_desc, centry_string(centry, mem_ctx));
+		(*info)[i].acct_name = centry_string(centry, (*info));
+		(*info)[i].acct_desc = centry_string(centry, (*info));
 		(*info)[i].rid = centry_uint32(centry);
 	}
 
diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
index f50fb8f..6f7cb07 100644
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -155,9 +155,13 @@ NTSTATUS rpc_enum_dom_groups(TALLOC_CTX *mem_ctx,
 		for (g = 0; g < count; g++) {
 			struct wb_acct_info *i = &info[num_info + g];
 
-			fstrcpy(i->acct_name,
+			i->acct_name = talloc_strdup(info,
 				sam_array->entries[g].name.string);
-			fstrcpy(i->acct_desc, "");
+			if (i->acct_name == NULL) {
+				TALLOC_FREE(info);
+				return NT_STATUS_NO_MEMORY;
+			}
+			i->acct_desc = NULL;
 			i->rid = sam_array->entries[g].idx;
 		}
 
@@ -217,9 +221,13 @@ NTSTATUS rpc_enum_local_groups(TALLOC_CTX *mem_ctx,
 		for (g = 0; g < count; g++) {
 			struct wb_acct_info *i = &info[num_info + g];
 
-			fstrcpy(i->acct_name,
+			i->acct_name = talloc_strdup(info,
 				sam_array->entries[g].name.string);
-			fstrcpy(i->acct_desc, "");
+			if (i->acct_name == NULL) {
+				TALLOC_FREE(info);
+				return NT_STATUS_NO_MEMORY;
+			}
+			i->acct_desc = NULL;
 			i->rid = sam_array->entries[g].idx;
 		}
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list