[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Tue May 9 02:59:02 UTC 2023


The branch, master has been updated
       via  6206e15b4de winbind: Fix "wbinfo -u" on a Samba AD DC with >1000 users
       via  f633389f36e winbind: Test wbinfo -u with more than 1000 users
       via  5ac65fdf9ac build:wafsamba: Fix TypeError in read_submodule_status()
       via  1dbdeaa8d7f gp: get_gpo() should re-raise the Exception, not return
       via  9755206f6dd s4:ntvfs:posix: avoid parsing empty blob in posix_eadb_add_list()
       via  46ae5568fa7 lib:ldb: do not offset against NULL pointer in ldb_ldif_read()
      from  5fcb675a8b0 s4/scripting: fix % len(res) was in the wrong place

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


- Log -----------------------------------------------------------------
commit 6206e15b4de0ba67d713124c2be353dabf3878c8
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Apr 26 17:19:29 2023 +0200

    winbind: Fix "wbinfo -u" on a Samba AD DC with >1000 users
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15366
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Tue May  9 02:58:45 UTC 2023 on atb-devel-224

commit f633389f36e79d3e772777ad7ca13012e3616273
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Apr 27 12:25:24 2023 +0200

    winbind: Test wbinfo -u with more than 1000 users
    
    winbind asks dcerpc_samr_LookupRids in one batch, where samr.idl has
    
    	NTSTATUS samr_LookupRids(
    		[in,ref]      policy_handle *domain_handle,
    		[in,range(0,1000)] uint32 num_rids,
    		[in,size_is(1000),length_is(num_rids)] uint32 rids[],
    		[out,ref]     lsa_Strings *names,
    		[out,ref]     samr_Ids *types
    		);
    
    limiting num_rids to 1000 entries. Test this.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15366
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 5ac65fdf9acb286a152032cc8913b5ce28fe30fc
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Thu May 4 15:25:31 2023 +1200

    build:wafsamba: Fix TypeError in read_submodule_status()
    
        parts = l.split(" ")
                ^^^^^^^^^^^^
    TypeError: a bytes-like object is required, not 'str'
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 1dbdeaa8d7fcfa4b620bbd24e457ee7f2e6c132d
Author: David Mulder <dmulder at samba.org>
Date:   Fri Apr 28 07:37:31 2023 -0600

    gp: get_gpo() should re-raise the Exception, not return
    
    If we return from this failure, then `new_gpo` is
    set to `None` and we will fail in some obscure
    way within a CSE later (since we append `None` to
    the GPO list). Instead, re-raise the Exception so
    we see that an error happened when fetching the
    GPO.
    
    Signed-off-by: David Mulder <dmulder at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 9755206f6dde7ee4f9852bbd81cb79f4457faf86
Author: Dmitry Antipov <dantipov at cloudlinux.com>
Date:   Tue May 2 13:45:01 2023 +0300

    s4:ntvfs:posix: avoid parsing empty blob in posix_eadb_add_list()
    
    Strictly speaking, this is not a bug because parsing loop will just skip
    an empty ({NULL}, 0) blob. But it's better to avoid this case because
    UBSan (as of clang-17 at least) may complain on such a parsing attempt:
    
    source4/ntvfs/posix/posix_eadb.c:56:62: runtime error: applying zero offset to null pointer
        #0 0x7f9d71ce7b2a in posix_eadb_add_list source4/ntvfs/posix/posix_eadb.c:56
        #1 0x7f9d71ce7b2a in push_xattr_blob_tdb_raw source4/ntvfs/posix/posix_eadb.c:178
        #2 0x7f9d71cec1f5 in py_wrap_setxattr source4/ntvfs/posix/python/pyposix_eadb.c:64
        #3 0x7f9d88bd4507 in cfunction_call (/lib64/libpython3.11.so.1.0+0x1d4507)
        [... a lot of Python calls skipped...]
    
    Signed-off-by: Dmitry Antipov <dantipov at cloudlinux.com>
    Reviewed-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 46ae5568fa7b9a96018d0eedadee6400632112ba
Author: Dmitry Antipov <dantipov at cloudlinux.com>
Date:   Tue May 2 13:43:54 2023 +0300

    lib:ldb: do not offset against NULL pointer in ldb_ldif_read()
    
    Fix the following error observed running samba.test.registry
    compiled with clang-17 and UBsan:
    
    lib/ldb/common/ldb_ldif.c:881:9: runtime error: applying non-zero offset 137438953440 to null pointer
        #0 0x7faa0eb3932f in ldb_ldif_read lib/ldb/common/ldb_ldif.c:881
        #1 0x7faa0eb3aec6 in ldb_ldif_read_string lib/ldb/common/ldb_ldif.c:1004
        #2 0x7faa077ed759 in dsdb_set_schema_from_ldif source4/dsdb/schema/schema_set.c:1113
        #3 0x7faa068fcbbf in py_dsdb_set_schema_from_ldif source4/dsdb/pydsdb.c:929
        #4 0x7faa1d1d4507 in cfunction_call (/lib64/libpython3.11.so.1.0+0x1d4507)
        [... a lot of Python calls skipped...]
    
    I.e. number of elements should be checked against zero
    before making an attempt to access an element by index.
    
    Signed-off-by: Dmitry Antipov <dantipov at cloudlinux.com>
    Reviewed-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

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

Summary of changes:
 buildtools/wafsamba/samba_git.py               |   1 +
 lib/ldb/common/ldb_ldif.c                      |   8 +-
 python/samba/gp/gpclass.py                     |   2 +-
 source3/script/tests/test_wbinfo_u_large_ad.sh |  28 +++++++
 source3/winbindd/winbindd_samr.c               | 102 +++++++++++++++----------
 source4/ntvfs/posix/posix_eadb.c               |  22 +++---
 source4/selftest/tests.py                      |   5 ++
 7 files changed, 114 insertions(+), 54 deletions(-)
 create mode 100755 source3/script/tests/test_wbinfo_u_large_ad.sh


Changeset truncated at 500 lines:

diff --git a/buildtools/wafsamba/samba_git.py b/buildtools/wafsamba/samba_git.py
index 09a204f1f4f..fe540ecccff 100644
--- a/buildtools/wafsamba/samba_git.py
+++ b/buildtools/wafsamba/samba_git.py
@@ -43,6 +43,7 @@ def read_submodule_status(path, env=None):
         cwd=path)
     (stdout, stderr) = p.communicate(None)
     for l in stdout.splitlines():
+        l = l.decode('utf-8')
         l = l.rstrip()
         status = l[0]
         l = l[1:]
diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c
index 748e44ed2b9..96237dd0abf 100644
--- a/lib/ldb/common/ldb_ldif.c
+++ b/lib/ldb/common/ldb_ldif.c
@@ -878,12 +878,12 @@ struct ldb_ldif *ldb_ldif_read(struct ldb_context *ldb,
 			continue;
 		}
 
-		el = &msg->elements[msg->num_elements-1];
-
 		a = ldb_schema_attribute_by_name(ldb, attr);
+		el = (msg->num_elements > 0
+		      ? &msg->elements[msg->num_elements - 1]
+		      : NULL);
 
-		if (msg->num_elements > 0 && ldb_attr_cmp(attr, el->name) == 0 &&
-		    flags == el->flags) {
+		if (el && ldb_attr_cmp(attr, el->name) == 0 && flags == el->flags) {
 			/* its a continuation */
 			el->values =
 				talloc_realloc(msg->elements, el->values,
diff --git a/python/samba/gp/gpclass.py b/python/samba/gp/gpclass.py
index 4992f17eeac..6083a0f8029 100644
--- a/python/samba/gp/gpclass.py
+++ b/python/samba/gp/gpclass.py
@@ -618,7 +618,7 @@ def get_gpo(samdb, gpo_dn):
                            controls=['sd_flags:1:%d' % sd_flags])
     except Exception:
         log.error('Failed to fetch gpo object with nTSecurityDescriptor')
-        return
+        raise
     if res.count != 1:
         raise ldb.LdbError(ldb.ERR_NO_SUCH_OBJECT,
                            'get_gpo: search failed')
diff --git a/source3/script/tests/test_wbinfo_u_large_ad.sh b/source3/script/tests/test_wbinfo_u_large_ad.sh
new file mode 100755
index 00000000000..ab5f0ca1f6a
--- /dev/null
+++ b/source3/script/tests/test_wbinfo_u_large_ad.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+LDBMODIFY="$VALGRIND ${LDBMODIFY:-$BINDIR/ldbmodify} $CONFIGURATION"
+LDBSEARCH="$VALGRIND ${LDBSEARCH:-$BINDIR/ldbsearch} $CONFIGURATION"
+WBINFO="$VALGRIND ${WBINFO:-$BINDIR/wbinfo} $CONFIGURATION"
+
+NUM_USERS=1234
+
+BASE_DN=$($LDBSEARCH -H ldap://$DC_SERVER -b "" --scope=base defaultNamingContext | awk '/^defaultNamingContext/ {print $2}')
+
+incdir=$(dirname $0)/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+seq -w 1 "$NUM_USERS" |
+    xargs -INUM echo -e "dn:cn=large_ad_NUM,cn=users,$BASE_DN\nchangetype:add\nobjectclass:user\nsamaccountname:large_ad_NUM\n" |
+    $LDBMODIFY -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+
+testit_grep_count \
+    "Make sure $NUM_USERS $DOMAIN users are returned" \
+    "$DOMAIN/large_ad_" \
+    "$NUM_USERS" \
+    ${WBINFO} -u || failed=$(expr $failed + 1)
+
+seq -w 1 "$NUM_USERS" |
+    xargs -INUM echo -e "dn:cn=large_ad_NUM,cn=users,$BASE_DN\nchangetype:delete\n" |
+    $LDBMODIFY -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD"
+
+testok $0 $failed
diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index ebf9c24b9e4..92dd1851abd 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -914,8 +914,6 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 	struct rpc_pipe_client *samr_pipe = NULL;
 	struct dcerpc_binding_handle *h = NULL;
 	struct policy_handle dom_pol = { .handle_type = 0, };
-	struct lsa_Strings lsa_names = { .count = 0, };
-	struct samr_Ids samr_types = { .count = 0, };
 	enum lsa_SidType *types = NULL;
 	char **names = NULL;
 	const char *domain_name = NULL;
@@ -997,49 +995,73 @@ again:
 	}
 	h = samr_pipe->binding_handle;
 
-	status = dcerpc_samr_LookupRids(
-		h,
-		tmp_ctx,
-		&dom_pol,
-		num_rids,
-		rids,
-		&lsa_names,
-		&samr_types,
-		&result);
-
-	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
-		retry = true;
-		goto again;
-	}
+	/*
+	 * Magic number 1000 comes from samr.idl
+	 */
 
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_DEBUG("dcerpc_samr_LookupRids failed: %s\n",
-			  nt_errstr(status));
-		goto fail;
-	}
-	if (!NT_STATUS_IS_OK(result) &&
-	    !NT_STATUS_EQUAL(result, STATUS_SOME_UNMAPPED)) {
-		DBG_DEBUG("dcerpc_samr_LookupRids resulted in %s\n",
-			  nt_errstr(result));
-		status = result;
-		goto fail;
-	}
+	for (i = 0; i < num_rids; i += 1000) {
+		uint32_t num_lookup_rids = MIN(num_rids - i, 1000);
+		struct lsa_Strings lsa_names = {
+			.count = 0,
+		};
+		struct samr_Ids samr_types = {
+			.count = 0,
+		};
+		uint32_t j;
+
+		status = dcerpc_samr_LookupRids(h,
+						tmp_ctx,
+						&dom_pol,
+						num_lookup_rids,
+						&rids[i],
+						&lsa_names,
+						&samr_types,
+						&result);
+
+		if (!retry &&
+		    reset_connection_on_error(domain, samr_pipe, status)) {
+			retry = true;
+			goto again;
+		}
 
-	for (i=0; i<num_rids; i++) {
-		types[i] = samr_types.ids[i];
-		names[i] = talloc_move(
-			names,
-			discard_const_p(char *, &lsa_names.names[i].string));
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_DEBUG("dcerpc_samr_LookupRids failed: %s\n",
+				  nt_errstr(status));
+			goto fail;
+		}
+		if (!NT_STATUS_IS_OK(result) &&
+		    !NT_STATUS_EQUAL(result, STATUS_SOME_UNMAPPED)) {
+			DBG_DEBUG("dcerpc_samr_LookupRids resulted in %s\n",
+				  nt_errstr(result));
+			status = result;
+			goto fail;
+		}
 
-		if (names[i] != NULL) {
-			char *normalized = NULL;
-			NTSTATUS nstatus = normalize_name_map(
-				names, domain_name, names[i], &normalized);
-			if (NT_STATUS_IS_OK(nstatus) ||
-			    NT_STATUS_EQUAL(nstatus, NT_STATUS_FILE_RENAMED)) {
-				names[i] = normalized;
+		for (j = 0; j < num_lookup_rids; j++) {
+			uint32_t dst = i + j;
+
+			types[dst] = samr_types.ids[j];
+			names[dst] = talloc_move(
+				names,
+				discard_const_p(char *,
+						&lsa_names.names[j].string));
+			if (names[dst] != NULL) {
+				char *normalized = NULL;
+				NTSTATUS nstatus =
+					normalize_name_map(names,
+							   domain_name,
+							   names[dst],
+							   &normalized);
+				if (NT_STATUS_IS_OK(nstatus) ||
+				    NT_STATUS_EQUAL(nstatus,
+						    NT_STATUS_FILE_RENAMED)) {
+					names[dst] = normalized;
+				}
 			}
 		}
+
+		TALLOC_FREE(samr_types.ids);
+		TALLOC_FREE(lsa_names.names);
 	}
 
 done:
diff --git a/source4/ntvfs/posix/posix_eadb.c b/source4/ntvfs/posix/posix_eadb.c
index e08597c1c19..2d181fd42a4 100644
--- a/source4/ntvfs/posix/posix_eadb.c
+++ b/source4/ntvfs/posix/posix_eadb.c
@@ -37,7 +37,6 @@ static NTSTATUS posix_eadb_add_list(struct tdb_wrap *ea_tdb, TALLOC_CTX *ctx, co
 {
 	DATA_BLOB blob;
 	TALLOC_CTX *mem_ctx;
-	const char *s;
 	NTSTATUS status;
 	size_t len;
 
@@ -49,15 +48,20 @@ static NTSTATUS posix_eadb_add_list(struct tdb_wrap *ea_tdb, TALLOC_CTX *ctx, co
 
 	status = pull_xattr_blob_tdb_raw(ea_tdb, mem_ctx, XATTR_LIST_ATTR,
 				     fname, fd, 100, &blob);
-	if (!NT_STATUS_IS_OK(status)) {
-		blob = data_blob(NULL, 0);
-	}
-
-	for (s=(const char *)blob.data; s < (const char *)(blob.data+blob.length); s += strlen(s) + 1) {
-		if (strcmp(attr_name, s) == 0) {
-			talloc_free(mem_ctx);
-			return NT_STATUS_OK;
+	if (NT_STATUS_IS_OK(status)) {
+		const char *s;
+
+		for (s = (const char *)blob.data;
+		     s < (const char *)(blob.data + blob.length);
+		     s += strlen(s) + 1) {
+			if (strcmp(attr_name, s) == 0) {
+				talloc_free(mem_ctx);
+				return NT_STATUS_OK;
+			}
 		}
+	} else {
+		blob = data_blob(NULL, 0);
+		/* No need to parse an empty blob */
 	}
 
 	len = strlen(attr_name) + 1;
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 19764a14397..b9cbdfed223 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1012,6 +1012,11 @@ for env in ["nt4_dc", "nt4_member", "ad_dc", "ad_member", "chgdcpass", "rodc"]:
 
     planpythontestsuite(env + ":local", "samba.tests.ntlm_auth")
 
+plantestsuite(
+    "samba.wbinfo_u_large_ad.(ad_dc:local)",
+    "ad_dc:local",
+    [os.path.join(samba3srcdir, "script/tests/test_wbinfo_u_large_ad.sh")])
+
 for env in ["ktest"]:
     planpythontestsuite(env + ":local", "samba.tests.ntlm_auth_krb5")
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list