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 <v...@samba.org>
Date: Fri Oct 5 16:27:48 2018 +0200
nsswitch: Run nsswitch thread test
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Thu Nov 1 05:06:23 CET 2018 on sn-devel-144
commit b5ea7946f844ff6b9337fea57fd002b716595703
Author: Ralph Wuerthner <ralph.wuerth...@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.wuerth...@de.ibm.com>
Reviewed-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 988182c3b8dab6511c75622e3484c9ef71e6d0db
Author: Ralph Wuerthner <ralph.wuerth...@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.wuerth...@de.ibm.com>
Reviewed-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit e82b3ac0ae5b50993db3c85d77dd317f957beb7e
Author: Ralph Wuerthner <ralph.wuerth...@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.wuerth...@de.ibm.com>
Reviewed-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 2cfb58d7535e63c5ef9f3970ebaa189741b715cb
Author: Ralph Wuerthner <ralph.wuerth...@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.wuerth...@de.ibm.com>
Reviewed-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 8d14714cc5e4c2b439e973faf602afb2fc1e7488
Author: Samuel Cabrero <scabr...@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 <scabr...@suse.de>
Reviewed-by: Jeremy Allison <j...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@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
