The branch, master has been updated
via 00316255984 dsdb: Make a shallow copy of ldb_parse_tree in
operational module
via 3b51091c20a dsdb: Replace talloc_steal() with a shallow copy and
reference in dsdb_paged_results
via 1b68bd977af paged_results: add no memory checks in paged_search()
via c67534fe3ff selftest: Add test for combination of anr and
paged_results
via 8f4c1c67b4f vfs_aio_pthread: fix segfault if samba-tool ntacl get
from d23dd3e26c5 dsdb: Add tracing to dsdb_search_dn() similar to
gendb_search_v()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 003162559848ce45d4f5bd3fb66642960538120f
Author: Andrew Bartlett <abart...@samba.org>
Date: Wed Aug 2 14:13:00 2023 +1200
dsdb: Make a shallow copy of ldb_parse_tree in operational module
We should not be making modifications to caller memory. In
particular, this causes problems for logging of requests if the
original request becomes modified.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15442
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
Autobuild-User(master): Stefan Metzmacher <me...@samba.org>
Autobuild-Date(master): Wed Aug 2 12:10:20 UTC 2023 on atb-devel-224
commit 3b51091c20a3c807932bcc986ebb8a676e0ffe6a
Author: Andrew Bartlett <abart...@samba.org>
Date: Wed Aug 2 14:12:07 2023 +1200
dsdb: Replace talloc_steal() with a shallow copy and reference in
dsdb_paged_results
We should not be stealing caller memory like this, and while a
talloc_reference() is not much better, this combined with a
shallow copy should be a little better in terms of polite
memory management.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15442
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 1b68bd977af39263a71af2c6a314c5ccb29e348c
Author: Stefan Metzmacher <me...@samba.org>
Date: Tue Feb 8 00:41:54 2022 +0100
paged_results: add no memory checks in paged_search()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15442
Signed-off-by: Arvid Requate <requ...@univention.de>
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
[abart...@samba.org combination of two patches by the above authors]
commit c67534fe3ff1652dcf95eac2030778b066cdf7a4
Author: Andrew Bartlett <abart...@samba.org>
Date: Wed Aug 2 13:40:03 2023 +1200
selftest: Add test for combination of anr and paged_results
This combination was known to cause a segfault in Samba 4.13, fixed by
5f0590362c5c0c5ee20503a67467f9be2d50e73b in later versions.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14970
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 8f4c1c67b4f118a9a47b09ac7908cd3d969b19c2
Author: Jones Syue <joness...@qnap.com>
Date: Wed Aug 2 09:48:40 2023 +0800
vfs_aio_pthread: fix segfault if samba-tool ntacl get
If configured as AD DC and aio_pthread appended into 'vfs objects'[1],
run these commands would get segfault:
1. sudo samba-tool ntacl get .
2. sudo net vfs getntacl sysvol .
gdb said it goes through aio_pthread_openat_fn() @ vfs_aio_pthread.c[2],
and the fsp->conn->sconn->client is null (0x0).
'sconn->client' memory is allocated when a new connection is accpeted:
smbd_accept_connection > smbd_process > smbXsrv_client_create
While running local commands looks like it would not go through
smbXsrv_client_create so the 'client' is null, segfault might happen.
We should not dereference 'client->server_multi_channel_enabled',
if 'client' is null.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15441
[1] smb.conf example, samba-4.18.5, ubuntu 22.04.2
[global]
dns forwarder = 127.0.0.53
netbios name = U22-JONES-88X1
realm = U22-JONES-88X1.X88X1.JONES
server role = active directory domain controller
workgroup = X88X1
idmap_ldb:use rfc2307 = yes
vfs objects = dfs_samba4 acl_xattr aio_pthread
[sysvol]
path = /var/lib/samba/sysvol
read only = No
[netlogon]
path = /var/lib/samba/sysvol/u22-jones-88x1.x88x1.jones/scripts
read only = No
[2] gdb
(gdb) run /usr/local/samba/bin/samba-tool ntacl get .
Starting program: /usr/local/Python3/bin/python3
/usr/local/samba/bin/samba-tool ntacl get .
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".
Program received signal SIGSEGV, Segmentation fault.
0x00007fffd0eb809e in aio_pthread_openat_fn (handle=0x8d5cc0,
dirfsp=0x8c3070, smb_fname=0x18ab4f0, fsp=0x1af3550, flags=196608, mode=0)
at ../../source3/modules/vfs_aio_pthread.c:467
warning: Source file is more recent than executable.
467 if (fsp->conn->sconn->client->server_multi_channel_enabled)
{
(gdb) bt
at ../../source3/modules/vfs_aio_pthread.c:467
at ../../source3/smbd/pysmbd.c:320
---Type <return> to continue, or q <return> to quit---
(gdb) f
at ../../source3/modules/vfs_aio_pthread.c:467
467 if (fsp->conn->sconn->client->server_multi_channel_enabled)
{
(gdb) p fsp->conn->sconn->client
$1 = (struct smbXsrv_client *) 0x0
(gdb)
Signed-off-by: Jones Syue <joness...@qnap.com>
Reviewed-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/modules/vfs_aio_pthread.c | 3 +-
source4/dsdb/samdb/ldb_modules/operational.c | 93 +++++++++++++++++++++++---
source4/dsdb/samdb/ldb_modules/paged_results.c | 39 ++++++++++-
source4/dsdb/tests/python/vlv.py | 21 ++++++
4 files changed, 143 insertions(+), 13 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/vfs_aio_pthread.c
b/source3/modules/vfs_aio_pthread.c
index 428ae5f2a4c..0303ff04bc9 100644
--- a/source3/modules/vfs_aio_pthread.c
+++ b/source3/modules/vfs_aio_pthread.c
@@ -475,7 +475,8 @@ static int aio_pthread_openat_fn(vfs_handle_struct *handle,
aio_allow_open = false;
}
- if (fsp->conn->sconn->client->server_multi_channel_enabled) {
+ if (fsp->conn->sconn->client != NULL &&
+ fsp->conn->sconn->client->server_multi_channel_enabled) {
/*
* This module is not compatible with multi channel yet.
*/
diff --git a/source4/dsdb/samdb/ldb_modules/operational.c
b/source4/dsdb/samdb/ldb_modules/operational.c
index 8821765a703..5d87a16564f 100644
--- a/source4/dsdb/samdb/ldb_modules/operational.c
+++ b/source4/dsdb/samdb/ldb_modules/operational.c
@@ -1556,6 +1556,7 @@ struct operational_context {
struct ldb_request *req;
enum ldb_scope scope;
const char * const *attrs;
+ struct ldb_parse_tree *tree;
struct op_controls_flags* controls_flags;
struct op_attributes_operations *list_operations;
unsigned int list_operations_size;
@@ -1681,12 +1682,57 @@ static struct op_attributes_operations*
operation_get_op_list(TALLOC_CTX *ctx,
return list;
}
+struct operational_present_ctx {
+ const char *attr;
+ bool found_operational;
+};
+
+/*
+ callback to determine if an operational attribute (needing
+ replacement) is in use at all
+ */
+static int operational_present(struct ldb_parse_tree *tree, void
*private_context)
+{
+ struct operational_present_ctx *ctx = private_context;
+ switch (tree->operation) {
+ case LDB_OP_EQUALITY:
+ case LDB_OP_GREATER:
+ case LDB_OP_LESS:
+ case LDB_OP_APPROX:
+ if (ldb_attr_cmp(tree->u.equality.attr, ctx->attr) == 0) {
+ ctx->found_operational = true;
+ }
+ break;
+ case LDB_OP_SUBSTRING:
+ if (ldb_attr_cmp(tree->u.substring.attr, ctx->attr) == 0) {
+ ctx->found_operational = true;
+ }
+ break;
+ case LDB_OP_PRESENT:
+ if (ldb_attr_cmp(tree->u.present.attr, ctx->attr) == 0) {
+ ctx->found_operational = true;
+ }
+ break;
+ case LDB_OP_EXTENDED:
+ if (tree->u.extended.attr &&
+ ldb_attr_cmp(tree->u.extended.attr, ctx->attr) == 0) {
+ ctx->found_operational = true;
+ }
+ break;
+ default:
+ break;
+ }
+ return LDB_SUCCESS;
+}
+
+
static int operational_search(struct ldb_module *module, struct ldb_request
*req)
{
struct ldb_context *ldb;
struct operational_context *ac;
struct ldb_request *down_req;
const char **search_attrs = NULL;
+ struct operational_present_ctx ctx;
unsigned int i, a;
int ret;
@@ -1707,15 +1753,44 @@ static int operational_search(struct ldb_module
*module, struct ldb_request *req
ac->scope = req->op.search.scope;
ac->attrs = req->op.search.attrs;
- /* FIXME: We must copy the tree and keep the original
- * unmodified. SSS */
- /* replace any attributes in the parse tree that are
- searchable, but are stored using a different name in the
- backend */
+ ctx.found_operational = false;
+
+ /*
+ * find any attributes in the parse tree that are searchable,
+ * but are stored using a different name in the backend, so we
+ * only duplicate the memory when needed
+ */
for (i=0;i<ARRAY_SIZE(parse_tree_sub);i++) {
- ldb_parse_tree_attr_replace(req->op.search.tree,
- parse_tree_sub[i].attr,
- parse_tree_sub[i].replace);
+ ctx.attr = parse_tree_sub[i].attr;
+
+ ldb_parse_tree_walk(req->op.search.tree,
+ operational_present,
+ &ctx);
+ if (ctx.found_operational) {
+ break;
+ }
+ }
+
+ if (ctx.found_operational) {
+
+ ac->tree = ldb_parse_tree_copy_shallow(ac,
+ req->op.search.tree);
+
+ if (ac->tree == NULL) {
+ return ldb_operr(ldb);
+ }
+
+ /* replace any attributes in the parse tree that are
+ searchable, but are stored using a different name in the
+ backend */
+ for (i=0;i<ARRAY_SIZE(parse_tree_sub);i++) {
+ ldb_parse_tree_attr_replace(ac->tree,
+ parse_tree_sub[i].attr,
+ parse_tree_sub[i].replace);
+ }
+ } else {
+ /* Avoid allocating a copy if we do not need to */
+ ac->tree = req->op.search.tree;
}
ac->controls_flags = talloc(ac, struct op_controls_flags);
@@ -1795,7 +1870,7 @@ static int operational_search(struct ldb_module *module,
struct ldb_request *req
ret = ldb_build_search_req_ex(&down_req, ldb, ac,
req->op.search.base,
req->op.search.scope,
- req->op.search.tree,
+ ac->tree,
/* use new set of attrs if any */
search_attrs ==
NULL?req->op.search.attrs:search_attrs,
req->controls,
diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c
b/source4/dsdb/samdb/ldb_modules/paged_results.c
index 2063e84e157..83729b02c0b 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -681,6 +681,7 @@ static int paged_search(struct ldb_module *module, struct
ldb_request *req)
struct ldb_control *ext_ctrl;
struct ldb_control **controls;
static const char * const attrs[1] = { NULL };
+ void *ref = NULL;
if (paged_ctrl->size == 0) {
return LDB_ERR_OPERATIONS_ERROR;
@@ -705,9 +706,15 @@ static int paged_search(struct ldb_module *module, struct
ldb_request *req)
struct ldb_request *req_extended_dn;
struct ldb_extended_dn_control *ext_ctrl_data;
req_extended_dn = talloc_zero(req, struct ldb_request);
+ if (req_extended_dn == NULL) {
+ return ldb_module_oom(module);
+ }
req_extended_dn->controls = req->controls;
ext_ctrl_data = talloc_zero(req,
struct ldb_extended_dn_control);
+ if (ext_ctrl_data == NULL) {
+ return ldb_module_oom(module);
+ }
ext_ctrl_data->type = 1;
ret = ldb_request_add_control(req_extended_dn,
@@ -733,11 +740,37 @@ static int paged_search(struct ldb_module *module, struct
ldb_request *req)
return ret;
}
- ac->store->expr = talloc_steal(ac->store, req->op.search.tree);
+ /*
+ * LDB does not have a function to take a full copy of
+ * this, but at least take a shallow copy
+ */
+ ac->store->expr = ldb_parse_tree_copy_shallow(ac->store,
+
req->op.search.tree);
+
+ if (ac->store->expr == NULL) {
+ return ldb_operr(ldb);
+ }
+
+ /*
+ * As the above is only a shallow copy, take a
+ * reference to ensure the values are kept around
+ */
+ ref = talloc_reference(ac->store, req->op.search.tree);
+ if (ref == NULL) {
+ return ldb_module_oom(module);
+ }
ac->store->expr_str = ldb_filter_from_tree(ac->store,
req->op.search.tree);
- ac->store->attrs = paged_copy_attrs(ac->store,
- req->op.search.attrs);
+ if (ac->store->expr_str == NULL) {
+ return ldb_module_oom(module);
+ }
+ if (req->op.search.attrs != NULL) {
+ ac->store->attrs = paged_copy_attrs(ac->store,
+
req->op.search.attrs);
+ if (ac->store->attrs == NULL) {
+ return ldb_module_oom(module);
+ }
+ }
/* save it locally and remove it from the list */
/* we do not need to replace them later as we
diff --git a/source4/dsdb/tests/python/vlv.py b/source4/dsdb/tests/python/vlv.py
index 33b51557ed3..13fccba75c9 100644
--- a/source4/dsdb/tests/python/vlv.py
+++ b/source4/dsdb/tests/python/vlv.py
@@ -1754,6 +1754,27 @@ class PagedResultsTestsRW(PagedResultsTests):
(enum, estr) = e.args
self.assertEqual(enum, ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION)
+ def test_anr_paged(self):
+ """Testing behaviour with anr= searches and paged_results set.
+
+ A problematic combination, as anr involves filter rewriting
+
+ """
+ prefix = "anr"
+ num_users = 5
+ users = [self.create_user(i, num_users, prefix=prefix)
+ for i in range(num_users)]
+ expr =
f"(|(anr={prefix})(&(objectClass=user)(facsimileTelephoneNumber={prefix}*)))"
+
+ results, cookie = self.paged_search(expr, page_size=1)
+ self.assertEqual(len(results), 1)
+
+ results, cookie = self.paged_search(expr, page_size=2, cookie=cookie)
+ self.assertEqual(len(results), 2)
+
+ results, cookie = self.paged_search(expr, page_size=2, cookie=cookie)
+ self.assertEqual(len(results), 2)
+
if "://" not in host:
if os.path.isfile(host):
--
Samba Shared Repository
