Attachment 15952 Details for Bug 14364 – patch v1 with bug

[patch] patch v1 with bug

vlv-asq-v1.patch (text/plain), 7.15 KB, created by Andrew Bartlett on 2020-05-05 05:34:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2020-05-05 05:34:49 UTC

Size: 7.15 KB

patch

obsolete

>From b95e076d7c3ada4cc6556aaec745d3f45464d71c Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 5 May 2020 12:54:59 +1200
>Subject: [PATCH 1/4] vlv: Use strcmp(), not strncmp() checking the NULL
> terminated control OIDs
>
>The end result is the same, as sizeof() includes the trailing NUL, but this
>avoids having to think about that.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
>---
> source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>index 980177cb05e..31e64b4bd78 100644
>--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>@@ -682,8 +682,8 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
> 		if (control->oid == NULL) {
> 			break;
> 		}
>-		if (strncmp(control->oid, LDB_CONTROL_VLV_REQ_OID, sizeof(LDB_CONTROL_VLV_REQ_OID)) == 0 ||
>-		    strncmp(control->oid, LDB_CONTROL_SERVER_SORT_OID, sizeof(LDB_CONTROL_SERVER_SORT_OID)) == 0) {
>+		if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 ||
>+		    strcmp(control->oid, LDB_CONTROL_SERVER_SORT_OID) == 0) {
> 			continue;
> 		}
> 		new_controls[j] = talloc_steal(new_controls, control);
>-- 
>2.17.1
>
>
>From cb667057fb71be7928270747ed63c598f15959ce Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 5 May 2020 13:16:48 +1200
>Subject: [PATCH 2/4] selftest: Add test to confirm VLV interaction with ASQ
>
>Tested against Windows 1709.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/asq-vlv     |  1 +
> source4/dsdb/tests/python/asq.py | 26 ++++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
> create mode 100644 selftest/knownfail.d/asq-vlv
>
>diff --git a/selftest/knownfail.d/asq-vlv b/selftest/knownfail.d/asq-vlv
>new file mode 100644
>index 00000000000..fb791993e90
>--- /dev/null
>+++ b/selftest/knownfail.d/asq-vlv
>@@ -0,0 +1 @@
>+^samba4.asq.python\(ad_dc_default\).__main__.ASQLDAPTest.test_asq_vlv\(ad_dc_default\)
>\ No newline at end of file
>diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py
>index a32c9f40cd3..a0c34031a18 100644
>--- a/source4/dsdb/tests/python/asq.py
>+++ b/source4/dsdb/tests/python/asq.py
>@@ -162,6 +162,32 @@ class ASQLDAPTest(samba.tests.TestCase):
>                 self.assertIn(ldb.Dn(self.ldb, str(group)),
>                               self.members)
> 
>+    def test_asq_vlv(self):
>+        """Testing ASQ behaviour with VLV set.
>+
>+        ASQ is very strange, it turns a BASE search into a search for
>+        all the objects pointed to by the specified attribute,
>+        returning multiple entries!
>+
>+        """
>+
>+        sort_control = "server_sort:1:0:cn"
>+
>+        msgs = self.ldb.search(base=self.top_dn,
>+                               scope=ldb.SCOPE_BASE,
>+                               attrs=["objectGUID", "cn", "member"],
>+                               controls=["asq:1:member",
>+                                         sort_control,
>+                                         "vlv:1:20:20:11:0"])
>+        self.assertEqual(len(msgs), 20)
>+
>+        for msg in msgs:
>+            self.assertNotEqual(msg.dn, self.top_dn)
>+            self.assertIn(msg.dn, self.members2)
>+            for group in msg["member"]:
>+                self.assertIn(ldb.Dn(self.ldb, str(group)),
>+                              self.members)
>+
> if "://" not in url:
>     if os.path.isfile(url):
>         url = "tdb://%s" % url
>-- 
>2.17.1
>
>
>From 394c76e20912b8b0d4c249b15e98b67e23671b26 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 5 May 2020 12:55:57 +1200
>Subject: [PATCH 3/4] vlv: Do not re-ASQ search the results of an ASQ search
> with VLV
>
>This is a silly combination, but at least try and keep the results sensible
>and avoid a double-dereference.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=1436
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/asq-vlv                    |  1 -
> source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 11 +++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
> delete mode 100644 selftest/knownfail.d/asq-vlv
>
>diff --git a/selftest/knownfail.d/asq-vlv b/selftest/knownfail.d/asq-vlv
>deleted file mode 100644
>index fb791993e90..00000000000
>--- a/selftest/knownfail.d/asq-vlv
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba4.asq.python\(ad_dc_default\).__main__.ASQLDAPTest.test_asq_vlv\(ad_dc_default\)
>\ No newline at end of file
>diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>index 31e64b4bd78..d58a62482c9 100644
>--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>@@ -682,10 +682,21 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls)
> 		if (control->oid == NULL) {
> 			break;
> 		}
>+		/*
>+		 * Do not re-use VLV, nor the server-sort, both are
>+		 * already handled here.
>+		 */
> 		if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 ||
> 		    strcmp(control->oid, LDB_CONTROL_SERVER_SORT_OID) == 0) {
> 			continue;
> 		}
>+		/*
>+		 * ASQ changes everything, do not copy it down for the
>+		 * per-GUID search
>+		 */
>+		if (strcmp(control->oid, LDB_CONTROL_ASQ_OID) == 0) {
>+			continue;
>+		}
> 		new_controls[j] = talloc_steal(new_controls, control);
> 		j++;
> 	}
>-- 
>2.17.1
>
>
>From df47f820336c15a5fc02611c6a72c07b769dce1a Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 5 May 2020 16:34:11 +1200
>Subject: [PATCH 4/4] vlv: Another workaround for mixing ASQ and VLV
>
>This is essentially an alternative patch, but without the correct
>behaviour.  Instead this just avoids a segfault.
>
>Included in case we have something simialr again in
>another module.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=1436
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> .../dsdb/samdb/ldb_modules/vlv_pagination.c   | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>index d58a62482c9..720b5e95638 100644
>--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
>@@ -442,10 +442,21 @@ static int vlv_results(struct vlv_context *ac)
> 			ret = vlv_search_by_dn_guid(ac->module, ac, &result, guid,
> 						    ac->req->op.search.attrs);
> 
>-			if (ret == LDAP_NO_SUCH_OBJECT) {
>-				/* The thing isn't there, which we quietly
>-				   ignore and go on to send an extra one
>-				   instead. */
>+			if (ret == LDAP_NO_SUCH_OBJECT
>+			    || result->count != 1) {
>+				/*
>+				 * The thing isn't there, which we quietly
>+				 * ignore and go on to send an extra one
>+				 * instead.
>+				 *
>+				 * result->count == 0 or > 1 can only
>+				 * happen if ASQ (which breaks all the
>+				 * rules) is somehow invoked (as this
>+				 * is a BASE search).
>+				 *
>+				 * (We skip the ASQ cookie for the
>+				 * GUID searches)
>+				 */
> 				if (last_i < ac->store->num_entries - 1) {
> 					last_i++;
> 				}
>-- 
>2.17.1
>

Actions: View

Attachments on bug 14364: 15951 | 15952 | 15983 | 15987 | 15998 | 16002 | 16003 | 16004 | 16005 | 16007 | 16008 | 16009 | 16056 | 16067 | 16089