Attachment 15027 Details for Bug 13836 – Backported patch for 4.10

[patch] Backported patch for 4.10

patch-4.10.txt (text/plain), 7.58 KB, created by Garming Sam on 2019-04-01 00:29:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Garming Sam

Created: 2019-04-01 00:29:05 UTC

Size: 7.58 KB

patch

obsolete

>From 0787802d313013fa3387149ea9cad7819a23e541 Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Mon, 25 Mar 2019 13:13:33 +1300
>Subject: [PATCH 1/2] ldb: cmocka test for empty attributes bug
>
>Cmocka test exposing LDB bug where a request with an empty attributes
>list returns a response containing all attributes.  The bug is in the
>ACL module and will be fixed in the next commit.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13836
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 24efa3ca5399d5cf538c3be504014a954685f1ed)
>---
> selftest/knownfail.d/dsdb        |  1 +
> source4/dsdb/common/tests/dsdb.c | 93 ++++++++++++++++++++++++++++++++
> source4/selftest/tests.py        |  3 ++
> source4/torture/wscript_build    |  9 ++++
> 4 files changed, 106 insertions(+)
> create mode 100644 selftest/knownfail.d/dsdb
> create mode 100644 source4/dsdb/common/tests/dsdb.c
>
>diff --git a/selftest/knownfail.d/dsdb b/selftest/knownfail.d/dsdb
>new file mode 100644
>index 00000000000..7a3a314778b
>--- /dev/null
>+++ b/selftest/knownfail.d/dsdb
>@@ -0,0 +1 @@
>+samba4.dsdb.no_attrs
>diff --git a/source4/dsdb/common/tests/dsdb.c b/source4/dsdb/common/tests/dsdb.c
>new file mode 100644
>index 00000000000..b38dee1c262
>--- /dev/null
>+++ b/source4/dsdb/common/tests/dsdb.c
>@@ -0,0 +1,93 @@
>+/*
>+   Unix SMB/CIFS implementation.
>+
>+   Test DSDB search
>+
>+   Copyright (C) Andrew Bartlet <abartlet@samba.org> 2019
>+
>+   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 "includes.h"
>+#include <ldb_module.h>
>+#include "ldb_wrap.h"
>+#include "param/param.h"
>+#include "param/loadparm.h"
>+#include "torture/smbtorture.h"
>+#include "torture/dsdb_proto.h"
>+#include "auth/auth.h"
>+
>+bool torture_ldb_no_attrs(struct torture_context *torture)
>+{
>+	struct ldb_context *ldb;
>+	int ret;
>+	struct ldb_request *req;
>+	struct ldb_result *ctx;
>+	struct ldb_dn *dn;
>+	const char *attrs[] = { NULL };
>+
>+	struct auth_session_info *session;
>+	struct dom_sid *domain_sid = NULL;
>+	const char *path;
>+
>+	path = lpcfg_private_path(NULL, torture->lp_ctx, "sam.ldb");
>+	torture_assert(torture, path != NULL,
>+		       "Couldn't find sam.ldb. Run with -s $SERVERCONFFILE");
>+
>+	domain_sid = dom_sid_parse_talloc(NULL, SID_BUILTIN);
>+	session = admin_session(NULL, torture->lp_ctx, domain_sid);
>+	ldb = ldb_wrap_connect(torture, torture->ev, torture->lp_ctx,
>+			       path, session, NULL, 0);
>+	torture_assert(torture, ldb, "Failed to connect to LDB target");
>+
>+	ctx = talloc_zero(ldb, struct ldb_result);
>+
>+	dn = ldb_get_default_basedn(ldb);
>+	ldb_dn_add_child_fmt(dn, "cn=users");
>+	ret = ldb_build_search_req(&req, ldb, ctx, dn, LDB_SCOPE_SUBTREE,
>+				   "(objectClass=*)", attrs, NULL,
>+				   ctx, ldb_search_default_callback, NULL);
>+	torture_assert(torture, ret == LDB_SUCCESS,
>+		       "Failed to build search request");
>+	ldb_req_mark_untrusted(req);
>+
>+	ret = ldb_request(ldb, req);
>+	torture_assert(torture, ret == LDB_SUCCESS, ldb_errstring(ldb));
>+
>+	ret = ldb_wait(req->handle, LDB_WAIT_ALL);
>+	torture_assert(torture, ret == LDB_SUCCESS, ldb_errstring(ldb));
>+
>+	torture_assert(torture, ctx->count > 0, "Users container empty");
>+	torture_assert_int_equal(torture, ctx->msgs[0]->num_elements, 0,
>+				 "Attributes returned for request "
>+				 "with empty attribute list");
>+
>+	return true;
>+}
>+
>+NTSTATUS torture_dsdb_init(TALLOC_CTX *mem_ctx)
>+{
>+	struct torture_suite *suite = torture_suite_create(mem_ctx, "dsdb");
>+
>+	if (suite == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+	torture_suite_add_simple_test(suite, "no_attrs", torture_ldb_no_attrs);
>+
>+	suite->description = talloc_strdup(suite, "DSDB tests");
>+
>+	torture_register_suite(mem_ctx, suite);
>+
>+	return NT_STATUS_OK;
>+}
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index b8132086ef8..bb35de154cb 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -150,6 +150,9 @@ for options in ['-U"$USERNAME%$PASSWORD"']:
> for t in smbtorture4_testsuites("ldap."):
>     plansmbtorture4testsuite(t, "ad_dc_ntvfs", '-U"$USERNAME%$PASSWORD" //$SERVER_IP/_none_')
> 
>+for t in smbtorture4_testsuites("dsdb."):
>+    plansmbtorture4testsuite(t, "ad_dc:local", "localhost")
>+
> ldbdir = os.path.join(srcdir(), "lib/ldb")
> # Don't run LDB tests when using system ldb, as we won't have ldbtest installed
> if os.path.exists(os.path.join(samba4bindir, "ldbtest")):
>diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build
>index 8d46d7355bc..4dcb9608b1d 100644
>--- a/source4/torture/wscript_build
>+++ b/source4/torture/wscript_build
>@@ -336,3 +336,12 @@ bld.SAMBA_BINARY('locktest',
> 	deps='popt POPT_SAMBA POPT_CREDENTIALS samba-util LIBCLI_SMB samba-hostconfig param_options',
> 	)
> 
>+bld.SAMBA_MODULE('TORTURE_DSDB',
>+	source="../../source4/dsdb/common/tests/dsdb.c",
>+	autoproto='dsdb_proto.h',
>+	subsystem='smbtorture',
>+	init_function='torture_dsdb_init',
>+	deps="TORTURE_UTIL samba-util",
>+	internal_module=True,
>+	enabled=bld.PYTHON_BUILD_IS_ENABLED()
>+	)
>-- 
>2.17.1
>
>
>From 877fe09f2801154b393199b106ec2e6bf1b04ac1 Mon Sep 17 00:00:00 2001
>From: Garming Sam <garming@catalyst.net.nz>
>Date: Wed, 13 Mar 2019 10:52:19 +1300
>Subject: [PATCH 2/2] acl_read: Fix regression caused by
> db15fcfa899e1fe4d6994f68ceb299921b8aa6f1 for empty lists
>
>The original code never dereferenced attrs and only added "*" if attrs
>was NULL (not if attrs[0] was NULL).
>
>This causes significant performance issues with the new paged_results
>module introduced for 4.10 as the initial GUID search requests no
>attributes. This GUID search turns into a search for "*" and ends up
>allocating memory for the entire database.
>
>This never appears to cause changes in the final result set, only
>intermediate processing.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13836
>
>Signed-off-by: Garming Sam <garming@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Fri Mar 29 18:37:29 UTC 2019 on sn-devel-144
>
>(cherry picked from commit a2b1970a37836e46d6c9eb6bda9bd20185de96ce)
>---
> selftest/knownfail.d/dsdb                 | 1 -
> source4/dsdb/samdb/ldb_modules/acl_read.c | 3 ---
> 2 files changed, 4 deletions(-)
> delete mode 100644 selftest/knownfail.d/dsdb
>
>diff --git a/selftest/knownfail.d/dsdb b/selftest/knownfail.d/dsdb
>deleted file mode 100644
>index 7a3a314778b..00000000000
>--- a/selftest/knownfail.d/dsdb
>+++ /dev/null
>@@ -1 +0,0 @@
>-samba4.dsdb.no_attrs
>diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
>index 6ab4780a196..9d93f671420 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
>@@ -796,9 +796,6 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req)
> 	if (attrs == NULL) {
> 		all_attrs = true;
> 		attrs = _all_attrs;
>-	} else if (attrs[0] == NULL) {
>-		all_attrs = true;
>-		attrs = _all_attrs;
> 	} else if (ldb_attr_in_list(attrs, "*")) {
> 		all_attrs = true;
> 	}
>-- 
>2.17.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 13836: 14927 | 15024 | 15025 | 15026 | 15027