[PATCH] Hold a read lock during the samba_dsdb stack init() phase
Andrew Bartlett
abartlet at samba.org
Wed Apr 11 23:37:35 UTC 2018
On Tue, 2018-04-10 at 22:13 +1200, Andrew Bartlett via samba-technical
wrote:
> The init_module hook is not locked by ldb, as the module may need to do
> things, but due to the multi-db nature of Samba's stack we need to lock
> it, so we do so in samba_dsdb.
>
> This should/may avoid lock ordering issues that show up as
> prepare_commit locking failures. (These seemed to show up a lot while
> I was trying to merge the LMDB patches, but I've seen them without so
> they are unrelated).
>
> CI results will be here:
>
> https://gitlab.com/catalyst-samba/samba/pipelines/20250607
>
> Please review and push if the CI indicates.
The CI is now good on the updated set:
https://gitlab.com/catalyst-samba/samba/pipelines/20322042
https://gitlab.com/catalyst-samba/samba/pipelines/20321956
The set of preparatory patches ready, excluding the LMDB patches
waiting on Metze is this set (attached):
https://gitlab.com/catalyst-samba/samba/commits/lmdb-pre-7
Please review and push!
Thanks,
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
-------------- next part --------------
From 9fa7d7b28be0bca9599b1299950f17cdb704b4f0 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 22:49:31 +1200
Subject: [PATCH 01/17] dsdb: Check for userPassword support after loading the
databases
The net result of this is only that userPassword values (which were
world readable when set) would still be visible after userPassword
started setting the main DB password.
In AD, those values become hidden once the dSHeuristics bit is set,
but Samba lost that when fixing a performance issue with
f26a2845bd42e580ddeaf0eecc9b46b823a0c6bc
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13378
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
selftest/knownfail.d/dsheuristics_userPassword | 1 +
source4/dsdb/samdb/ldb_modules/acl.c | 18 +++++++++++++++---
2 files changed, 16 insertions(+), 3 deletions(-)
create mode 100644 selftest/knownfail.d/dsheuristics_userPassword
diff --git a/selftest/knownfail.d/dsheuristics_userPassword b/selftest/knownfail.d/dsheuristics_userPassword
new file mode 100644
index 00000000000..6981255d9e9
--- /dev/null
+++ b/selftest/knownfail.d/dsheuristics_userPassword
@@ -0,0 +1 @@
+^samba4.ldap.passwords.python\(.*\).__main__.PasswordTests.test_modify_dsheuristics_userPassword
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index d750362c47f..8b1dcbeed51 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -108,8 +108,6 @@ static int acl_module_init(struct ldb_module *module)
NULL, "acl", "search", true);
ldb_module_set_private(module, data);
- data->userPassword_support = dsdb_user_password_support(module, module, NULL);
-
mem_ctx = talloc_new(module);
if (!mem_ctx) {
return ldb_oom(ldb);
@@ -180,7 +178,21 @@ static int acl_module_init(struct ldb_module *module)
done:
talloc_free(mem_ctx);
- return ldb_next_init(module);
+ ret = ldb_next_init(module);
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /*
+ * Check this after the modules have be initalised so we
+ * can actually read the backend DB.
+ */
+ data->userPassword_support
+ = dsdb_user_password_support(module,
+ module,
+ NULL);
+ return ret;
}
static int acl_allowedAttributes(struct ldb_module *module,
--
2.11.0
From 420074e5a5ff44bcb6e93998f2434f97227f9a5f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 22:47:03 +1200
Subject: [PATCH 02/17] dsdb: check for dSHeuristics more carefully
This check would pass if the dSHeuristics was treated as always being
000000000 for searches which is not enough, we must check for a value
of 000000001 (userPassword enabled).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13378
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
selftest/knownfail.d/dsheuristics_userPassword | 1 -
source4/dsdb/tests/python/passwords.py | 31 +++++++++++++++++++++-----
2 files changed, 25 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/dsheuristics_userPassword
diff --git a/selftest/knownfail.d/dsheuristics_userPassword b/selftest/knownfail.d/dsheuristics_userPassword
deleted file mode 100644
index 6981255d9e9..00000000000
--- a/selftest/knownfail.d/dsheuristics_userPassword
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldap.passwords.python\(.*\).__main__.PasswordTests.test_modify_dsheuristics_userPassword
diff --git a/source4/dsdb/tests/python/passwords.py b/source4/dsdb/tests/python/passwords.py
index c8c2b762a64..bbb8be1d2ca 100755
--- a/source4/dsdb/tests/python/passwords.py
+++ b/source4/dsdb/tests/python/passwords.py
@@ -985,7 +985,8 @@ userPassword: thatsAcomplPASS4
res = ldb1.search("cn=testuser,cn=users," + self.base_dn,
scope=SCOPE_BASE, attrs=["userPassword"])
- # userPassword cannot be read, despite the dsHeuristic setting
+ # userPassword cannot be read, it wasn't set, instead the
+ # password was
self.assertTrue(len(res) == 1)
self.assertFalse("userPassword" in res[0])
@@ -993,7 +994,15 @@ userPassword: thatsAcomplPASS4
ldb2 = SamDB(url=host, session_info=system_session(lp),
credentials=creds, lp=lp)
- # Set userPassword to be unreadable
+ res = ldb2.search("cn=testuser,cn=users," + self.base_dn,
+ scope=SCOPE_BASE, attrs=["userPassword"])
+
+ # Check on the new connection that userPassword was not stored
+ # from ldb1 or is not readable
+ self.assertTrue(len(res) == 1)
+ self.assertFalse("userPassword" in res[0])
+
+ # Set userPassword to be readable
# This setting does not affect this connection
ldb2.set_dsheuristics("000000000")
time.sleep(1)
@@ -1014,11 +1023,10 @@ userPassword: thatsAcomplPASS4
res = ldb2.search("cn=testuser,cn=users," + self.base_dn,
scope=SCOPE_BASE, attrs=["userPassword"])
- # userPassword can be read in this connection
- # This is regardless of the current dsHeuristics setting
+ # Check despite setting it with userPassword support disabled
+ # on this connection it should still not be readable
self.assertTrue(len(res) == 1)
- self.assertTrue("userPassword" in res[0])
- self.assertEquals(res[0]["userPassword"][0], "thatsAcomplPASS2")
+ self.assertFalse("userPassword" in res[0])
# Only password from ldb1 is the user's password
creds2 = Credentials()
@@ -1050,6 +1058,17 @@ userPassword: thatsAcomplPASS4
# Reset the test "dSHeuristics" (reactivate "userPassword" pwd changes)
self.ldb.set_dsheuristics("000000001")
+ ldb4 = SamDB(url=host, session_info=system_session(lp),
+ credentials=creds, lp=lp)
+
+ # Check that userPassword that was stored from ldb2
+ res = ldb4.search("cn=testuser,cn=users," + self.base_dn,
+ scope=SCOPE_BASE, attrs=["userPassword"])
+
+ # userPassword can be not be read
+ self.assertTrue(len(res) == 1)
+ self.assertFalse("userPassword" in res[0])
+
def test_zero_length(self):
# Get the old "minPwdLength"
minPwdLength = self.ldb.get_minPwdLength()
--
2.11.0
From 91e84f5f018e58b56b13bbb268f4d93ec102a8f0 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Mar 2018 13:42:17 +1300
Subject: [PATCH 03/17] ldb_wrap: Remove ldb_transaction_cancel_noerr from
ldb_wrap_fork_hook()
Writing to a TDB, without locks (these are per-process) in a forked child is never going to
end well, if a transaction is open at this point we have bigger problems.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb-samba/ldb_wrap.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/lib/ldb-samba/ldb_wrap.c b/lib/ldb-samba/ldb_wrap.c
index 9959b04ed95..8c3bf6f7bf3 100644
--- a/lib/ldb-samba/ldb_wrap.c
+++ b/lib/ldb-samba/ldb_wrap.c
@@ -329,20 +329,11 @@ int samba_ldb_connect(struct ldb_context *ldb, struct loadparm_context *lp_ctx,
}
/*
- when we fork() we need to make sure that any open ldb contexts have
- any open transactions cancelled (ntdb databases doesn't need reopening,
- as we don't use clear_if_first).
- */
+ call tdb_reopen_all() in case there is a TDB open so we are
+ not blocked from re-opening it inside ldb_tdb.
+*/
void ldb_wrap_fork_hook(void)
{
- struct ldb_wrap *w;
-
- for (w=ldb_wrap_list; w; w=w->next) {
- if (ldb_transaction_cancel_noerr(w->ldb) != LDB_SUCCESS) {
- smb_panic("Failed to cancel child transactions\n");
- }
- }
-
if (tdb_reopen_all(1) != 0) {
smb_panic("tdb_reopen_all failed\n");
}
--
2.11.0
From 4f0ff1f2e7e507df0e67d4c8af22eed058fb37d9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 9 Apr 2018 14:52:47 +1200
Subject: [PATCH 04/17] dsdb: Ensure to cancel the transaction if we fail to
save the prefixMap
This rare error case forgot to call ldb_transaction_cancel()
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/repl/replicated_objects.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c
index 0c44960cf5f..4c8890f0553 100644
--- a/source4/dsdb/repl/replicated_objects.c
+++ b/source4/dsdb/repl/replicated_objects.c
@@ -921,6 +921,7 @@ WERROR dsdb_replicated_objects_commit(struct ldb_context *ldb,
}
DEBUG(0,("Failed to save updated prefixMap: %s\n",
win_errstr(werr)));
+ ldb_transaction_cancel(ldb);
TALLOC_FREE(tmp_ctx);
return werr;
}
--
2.11.0
From 523fa3fe5ad244e5ac130ecf4686c39d319b53f1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 9 Apr 2018 17:51:57 +1200
Subject: [PATCH 05/17] dsdb: Do not create a transaction in partition_init()
This will allow us to lock the databases for read during all of the Samba init
hooks.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
.../dsdb/samdb/ldb_modules/partition_metadata.c | 59 ++++++++++------------
1 file changed, 27 insertions(+), 32 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
index e3ad0d8c6c2..3e60393a2c9 100644
--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
@@ -319,38 +319,6 @@ int partition_metadata_init(struct ldb_module *module)
return ret;
}
- /*
- * We need to fill in the sequence number from the DB, so we
- * need to get a lock over all the databases. We only read
- * from the main partitions, but write to metadata so to avoid
- * lock ordering we just get a transaction over the lot.
- */
- ret = partition_start_trans(module);
- if (ret != LDB_SUCCESS) {
- TALLOC_FREE(data->metadata);
- return ret;
- }
-
- ret = partition_metadata_set_sequence_number(module);
- if (ret != LDB_SUCCESS) {
- TALLOC_FREE(data->metadata);
- partition_del_trans(module);
- return ret;
- }
-
- ret = partition_prepare_commit(module);
- if (ret != LDB_SUCCESS) {
- TALLOC_FREE(data->metadata);
- partition_del_trans(module);
- return ret;
- }
-
- ret = partition_end_trans(module);
- if (ret != LDB_SUCCESS) {
- /* Nothing much we can do */
- TALLOC_FREE(data->metadata);
- }
-
return ret;
}
@@ -370,6 +338,13 @@ int partition_metadata_sequence_number(struct ldb_module *module, uint64_t *valu
return ret;
}
+ /*
+ * This means we will give a 0 until the first write
+ * tranaction, which is actually pretty reasonable.
+ *
+ * All modern databases will have the metadata.tdb from
+ * the time of the first transaction in provision anyway.
+ */
ret = partition_metadata_get_uint64(module,
LDB_METADATA_SEQ_NUM,
value,
@@ -410,6 +385,26 @@ int partition_metadata_sequence_number_increment(struct ldb_module *module, uint
return ret;
}
+ if (*value == 0) {
+ /*
+ * We are in a transaction now, so we can get the
+ * sequence number from the partitions.
+ */
+ ret = partition_metadata_set_sequence_number(module);
+ if (ret != LDB_SUCCESS) {
+ TALLOC_FREE(data->metadata);
+ partition_del_trans(module);
+ return ret;
+ }
+
+ ret = partition_metadata_get_uint64(module,
+ LDB_METADATA_SEQ_NUM,
+ value, 0);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+ }
+
(*value)++;
ret = partition_metadata_set_uint64(module, LDB_METADATA_SEQ_NUM, *value, false);
return ret;
--
2.11.0
From 55fc4a33ceffb0ab2e67556ec302ca6e9d5db8fc Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 9 Apr 2018 21:15:25 +1200
Subject: [PATCH 06/17] dsdb: Allow search before init() call in
encrypted_secrets
Simply do not decrypt anything until the init call is run.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/encrypted_secrets.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c b/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
index 87ec9e4eb53..ef69bb0831c 100644
--- a/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
+++ b/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
@@ -1365,7 +1365,7 @@ static int es_search_post_process(struct ldb_module *module,
/*
* Decrypt any encrypted secret attributes
*/
- if (data->encrypt_secrets) {
+ if (data && data->encrypt_secrets) {
int err = decrypt_secret_attributes(ldb, msg, data);
if (err != LDB_SUCCESS) {
return err;
--
2.11.0
From 3b48509120a7138d878d2cb503ef4be78dcfec52 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 9 Apr 2018 21:59:01 +1200
Subject: [PATCH 07/17] dsdb: Wait until a transaction starts to call
dsdb_schema_set_indices_and_attributes()
This avoids starting a transaction in schema_load_init() and allows it
to operate with a read lock held, which will avoid locking issues
(deadlock detected due to lock odering if we do not have a global
read lock).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
selftest/knownfail.d/dsdb_schema_attributes | 2 +
source4/dsdb/samdb/ldb_modules/schema_load.c | 60 +++++++---------------------
2 files changed, 17 insertions(+), 45 deletions(-)
create mode 100644 selftest/knownfail.d/dsdb_schema_attributes
diff --git a/selftest/knownfail.d/dsdb_schema_attributes b/selftest/knownfail.d/dsdb_schema_attributes
new file mode 100644
index 00000000000..4e9e7a05c94
--- /dev/null
+++ b/selftest/knownfail.d/dsdb_schema_attributes
@@ -0,0 +1,2 @@
+^samba.tests.dsdb_schema_attributes.samba.tests.dsdb_schema_attributes.SchemaAttributesTestCase.test_modify_at_attributes
+^samba.tests.dsdb_schema_attributes.samba.tests.dsdb_schema_attributes.SchemaAttributesTestCase.test_modify_at_indexlist
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 2099fac1159..55fb63a0f30 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -42,6 +42,12 @@ struct schema_load_private_data {
uint64_t schema_seq_num_read_lock;
uint64_t schema_seq_num_cache;
int tdb_seqnum;
+
+ /*
+ * Please write out the updated schema on the next transaction
+ * start
+ */
+ bool need_write;
};
static int dsdb_schema_from_db(struct ldb_module *module,
@@ -495,7 +501,6 @@ static int schema_load_init(struct ldb_module *module)
struct ldb_context *ldb = ldb_module_get_ctx(module);
struct schema_load_private_data *private_data;
int ret;
- bool need_write = false;
private_data = talloc_zero(module, struct schema_load_private_data);
if (private_data == NULL) {
@@ -510,50 +515,7 @@ static int schema_load_init(struct ldb_module *module)
return ret;
}
- ret = schema_load(ldb, module, &need_write);
-
- if (ret == LDB_SUCCESS && need_write) {
- TALLOC_CTX *frame = talloc_stackframe();
- struct dsdb_schema *schema = NULL;
-
- ret = ldb_transaction_start(ldb);
- if (ret != LDB_SUCCESS) {
- ldb_debug_set(ldb, LDB_DEBUG_FATAL,
- "schema_load_init: transaction start failed");
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- schema = dsdb_get_schema(ldb, frame);
- if (schema == NULL) {
- ldb_debug_set(ldb, LDB_DEBUG_FATAL,
- "schema_load_init: dsdb_get_schema failed");
- ldb_transaction_cancel(ldb);
- return LDB_ERR_OPERATIONS_ERROR;
- }
- ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
- SCHEMA_WRITE);
-
- TALLOC_FREE(frame);
-
- if (ret != LDB_SUCCESS) {
- ldb_asprintf_errstring(ldb, "Failed to write new "
- "@INDEXLIST and @ATTRIBUTES "
- "records for updated schema: %s",
- ldb_errstring(ldb));
- ldb_transaction_cancel(ldb);
- return ret;
- }
-
- ret = ldb_transaction_commit(ldb);
- if (ret != LDB_SUCCESS) {
- ldb_debug_set(ldb, LDB_DEBUG_FATAL,
- "schema_load_init: transaction commit failed");
- return LDB_ERR_OPERATIONS_ERROR;
- }
- }
-
-
- return ret;
+ return schema_load(ldb, module, &private_data->need_write);
}
static int schema_search(struct ldb_module *module, struct ldb_request *req)
@@ -586,6 +548,14 @@ static int schema_load_start_transaction(struct ldb_module *module)
"schema_load_init: dsdb_get_schema failed");
return LDB_ERR_OPERATIONS_ERROR;
}
+
+ if (private_data->need_write) {
+ ret = dsdb_schema_set_indices_and_attributes(ldb,
+ schema,
+ SCHEMA_WRITE);
+ private_data->need_write = false;
+ }
+
private_data->in_transaction++;
return ret;
--
2.11.0
From d4fdbdf2a51063ceb3296b4335564e91ef81fa6d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 12:51:49 +1200
Subject: [PATCH 08/17] selftest: Make a transaction before @INDEXLIST etc is
checked in dsdb_schema_attributes.py
This helps us remove the write to the database from the (soon to be
read locked) init code.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/tests/dsdb_schema_attributes.py | 16 ++++++++++++++++
selftest/knownfail.d/dsdb_schema_attributes | 2 --
2 files changed, 16 insertions(+), 2 deletions(-)
delete mode 100644 selftest/knownfail.d/dsdb_schema_attributes
diff --git a/python/samba/tests/dsdb_schema_attributes.py b/python/samba/tests/dsdb_schema_attributes.py
index 2bebbb569f2..c3b956f3b99 100644
--- a/python/samba/tests/dsdb_schema_attributes.py
+++ b/python/samba/tests/dsdb_schema_attributes.py
@@ -193,6 +193,14 @@ systemOnly: FALSE
samdb2 = samba.tests.connect_samdb(self.lp.samdb_url())
+ # We now only update the @ATTRIBUTES when a transaction happens
+ # rather than making a read of the DB do writes.
+ #
+ # This avoids locking issues and is more expected
+
+ samdb2.transaction_start()
+ samdb2.transaction_commit()
+
res = self.samdb.search(base="@ATTRIBUTES", scope=ldb.SCOPE_BASE,
attrs=["@TEST_EXTRA"])
self.assertEquals(len(res), 1)
@@ -220,6 +228,14 @@ systemOnly: FALSE
samdb2 = samba.tests.connect_samdb(self.lp.samdb_url())
+ # We now only update the @INDEXLIST when a transaction happens
+ # rather than making a read of the DB do writes.
+ #
+ # This avoids locking issues and is more expected
+
+ samdb2.transaction_start()
+ samdb2.transaction_commit()
+
res = self.samdb.search(base="@INDEXLIST", scope=ldb.SCOPE_BASE,
attrs=["@TEST_EXTRA"])
self.assertEquals(len(res), 1)
diff --git a/selftest/knownfail.d/dsdb_schema_attributes b/selftest/knownfail.d/dsdb_schema_attributes
deleted file mode 100644
index 4e9e7a05c94..00000000000
--- a/selftest/knownfail.d/dsdb_schema_attributes
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.tests.dsdb_schema_attributes.samba.tests.dsdb_schema_attributes.SchemaAttributesTestCase.test_modify_at_attributes
-^samba.tests.dsdb_schema_attributes.samba.tests.dsdb_schema_attributes.SchemaAttributesTestCase.test_modify_at_indexlist
--
2.11.0
From 8348c9916f0bc76900cc29820419d8ab99934a57 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 07:58:07 +1200
Subject: [PATCH 09/17] dsdb: Create rootdse_get_private_data()
This will get the private data on the first call, allowing that not to be
the init() hook.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/rootdse.c | 63 ++++++++++++++++++++++++--------
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c
index 0d486218c0b..0d4cbc60f24 100644
--- a/source4/dsdb/samdb/ldb_modules/rootdse.c
+++ b/source4/dsdb/samdb/ldb_modules/rootdse.c
@@ -863,11 +863,46 @@ static int rootdse_search(struct ldb_module *module, struct ldb_request *req)
return ldb_next_request(module, down_req);
}
+static struct rootdse_private_data *rootdse_get_private_data(struct ldb_module *module)
+{
+ void *priv = ldb_module_get_private(module);
+ struct rootdse_private_data *data = NULL;
+
+ if (priv != NULL) {
+ data = talloc_get_type_abort(priv,
+ struct rootdse_private_data);
+ }
+
+ if (data != NULL) {
+ return data;
+ }
+
+ data = talloc_zero(module, struct rootdse_private_data);
+ if (data == NULL) {
+ return NULL;
+ }
+
+ data->num_controls = 0;
+ data->controls = NULL;
+ data->num_partitions = 0;
+ data->partitions = NULL;
+ data->block_anonymous = true;
+
+ ldb_module_set_private(module, data);
+ return data;
+}
+
+
static int rootdse_register_control(struct ldb_module *module, struct ldb_request *req)
{
- struct rootdse_private_data *priv = talloc_get_type(ldb_module_get_private(module), struct rootdse_private_data);
+ struct rootdse_private_data *priv =
+ rootdse_get_private_data(module);
char **list;
+ if (priv == NULL) {
+ return ldb_module_oom(module);
+ }
+
list = talloc_realloc(priv, priv->controls, char *, priv->num_controls + 1);
if (!list) {
return ldb_oom(ldb_module_get_ctx(module));
@@ -886,9 +921,14 @@ static int rootdse_register_control(struct ldb_module *module, struct ldb_reques
static int rootdse_register_partition(struct ldb_module *module, struct ldb_request *req)
{
- struct rootdse_private_data *priv = talloc_get_type(ldb_module_get_private(module), struct rootdse_private_data);
+ struct rootdse_private_data *priv =
+ rootdse_get_private_data(module);
struct ldb_dn **list;
+ if (priv == NULL) {
+ return ldb_module_oom(module);
+ }
+
list = talloc_realloc(priv, priv->partitions, struct ldb_dn *, priv->num_partitions + 1);
if (!list) {
return ldb_oom(ldb_module_get_ctx(module));
@@ -924,28 +964,21 @@ static int rootdse_request(struct ldb_module *module, struct ldb_request *req)
static int rootdse_init(struct ldb_module *module)
{
int ret;
- struct ldb_context *ldb;
struct ldb_result *res;
- struct rootdse_private_data *data;
const char *attrs[] = { "msDS-Behavior-Version", NULL };
const char *ds_attrs[] = { "dsServiceName", NULL };
TALLOC_CTX *mem_ctx;
- ldb = ldb_module_get_ctx(module);
+ struct ldb_context *ldb
+ = ldb_module_get_ctx(module);
+
+ struct rootdse_private_data *data
+ = rootdse_get_private_data(module);
- data = talloc_zero(module, struct rootdse_private_data);
if (data == NULL) {
- return ldb_oom(ldb);
+ return ldb_module_oom(module);
}
- data->num_controls = 0;
- data->controls = NULL;
- data->num_partitions = 0;
- data->partitions = NULL;
- data->block_anonymous = true;
-
- ldb_module_set_private(module, data);
-
ldb_set_default_dns(ldb);
ret = ldb_next_init(module);
--
2.11.0
From 52278657e8b97fb5560ab9132ef043fb1356537e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 07:54:20 +1200
Subject: [PATCH 10/17] dsdb: Move ldb_set_default_dns() into
rootdse_get_private_data()
This call needs to be done at the very first chance, in this case
during the first call to the lock_read() hook, otherwise the
schema_data module can't find the schema.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/rootdse.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c
index 0d4cbc60f24..751fe15d1a1 100644
--- a/source4/dsdb/samdb/ldb_modules/rootdse.c
+++ b/source4/dsdb/samdb/ldb_modules/rootdse.c
@@ -867,6 +867,8 @@ static struct rootdse_private_data *rootdse_get_private_data(struct ldb_module *
{
void *priv = ldb_module_get_private(module);
struct rootdse_private_data *data = NULL;
+ struct ldb_context *ldb
+ = ldb_module_get_ctx(module);
if (priv != NULL) {
data = talloc_get_type_abort(priv,
@@ -889,6 +891,9 @@ static struct rootdse_private_data *rootdse_get_private_data(struct ldb_module *
data->block_anonymous = true;
ldb_module_set_private(module, data);
+
+ ldb_set_default_dns(ldb);
+
return data;
}
@@ -979,8 +984,6 @@ static int rootdse_init(struct ldb_module *module)
return ldb_module_oom(module);
}
- ldb_set_default_dns(ldb);
-
ret = ldb_next_init(module);
if (ret != LDB_SUCCESS) {
--
2.11.0
From a6729c4b616dc6376c6a417d321f19d06f7e50cd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 16:34:21 +1200
Subject: [PATCH 11/17] dsdb: Allow search before init() is called in
extended_dn_out
This matches the earlier check of p && p->normalise.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/extended_dn_out.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
index ad4603fbbb9..6a869d0c482 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
@@ -498,7 +498,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
continue;
}
- if (p->normalise) {
+ if (p && p->normalise) {
/* If we are also in 'normalise' mode, then
* fix the attribute names to be in the
* correct case */
--
2.11.0
From b0ba46c1e4bd1519e181af4b08024bf1fbdc3567 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 11:58:22 +1200
Subject: [PATCH 12/17] dsdb: Load schema during the read_lock() hook, not the
search
This should trigger slightly less often and is the more correct place, as
we only load it during the first lock when not in a transaction.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/schema_load.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 55fb63a0f30..9ed363a5973 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -518,16 +518,6 @@ static int schema_load_init(struct ldb_module *module)
return schema_load(ldb, module, &private_data->need_write);
}
-static int schema_search(struct ldb_module *module, struct ldb_request *req)
-{
- struct ldb_context *ldb = ldb_module_get_ctx(module);
-
- /* Try the schema refresh now */
- dsdb_get_schema(ldb, NULL);
-
- return ldb_next_request(module, req);
-}
-
static int schema_load_start_transaction(struct ldb_module *module)
{
struct schema_load_private_data *private_data =
@@ -632,8 +622,10 @@ static int schema_read_lock(struct ldb_module *module)
return ldb_next_read_lock(module);
}
+ private_data->in_read_transaction++;
+
if (private_data->in_transaction == 0 &&
- private_data->in_read_transaction == 0) {
+ private_data->in_read_transaction == 1) {
/*
* This appears to fail during the init path, so do not bother
* checking the return, and return 0 (reload schema).
@@ -643,8 +635,10 @@ static int schema_read_lock(struct ldb_module *module)
&schema_seq_num, 0);
private_data->schema_seq_num_read_lock = schema_seq_num;
+
+ /* Try the schema refresh now */
+ dsdb_get_schema(ldb_module_get_ctx(module), NULL);
}
- private_data->in_read_transaction++;
return ldb_next_read_lock(module);
@@ -673,7 +667,6 @@ static const struct ldb_module_ops ldb_schema_load_module_ops = {
.name = "schema_load",
.init_context = schema_load_init,
.extended = schema_load_extended,
- .search = schema_search,
.start_transaction = schema_load_start_transaction,
.end_transaction = schema_load_end_transaction,
.del_transaction = schema_load_del_transaction,
--
2.11.0
From 9a8aacc19a3c0534327d46f7e26f65ee7979fd8a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 12:29:18 +1200
Subject: [PATCH 13/17] dsdb: Rework schema reload during the read lock
Rather than refusing the reload based on making cached sequence numbers match
just load it once at the time the DB is globally locked, if required.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/schema_load.c | 69 ++++++++++++----------------
1 file changed, 29 insertions(+), 40 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 9ed363a5973..cfd88fec467 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -39,7 +39,6 @@ struct schema_load_private_data {
uint64_t in_transaction;
uint64_t in_read_transaction;
struct tdb_wrap *metadata;
- uint64_t schema_seq_num_read_lock;
uint64_t schema_seq_num_cache;
int tdb_seqnum;
@@ -201,16 +200,26 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
return schema;
}
- if (private_data->in_transaction > 0) {
-
+ if (schema != NULL) {
/*
- * If the refresh is not an expected part of a larger
- * transaction, then we don't allow a schema reload during a
- * transaction. This stops others from modifying our schema
- * behind our backs
+ * If we have a schema already (not in the startup)
+ * and we are in a read or write transaction, then
+ * avoid a schema reload, it can't have changed
*/
- if (ldb_get_opaque(ldb, "dsdb_schema_refresh_expected") != (void *)1) {
- return schema;
+ if (private_data->in_transaction > 0
+ || private_data->in_read_transaction > 0 ) {
+ /*
+ * If the refresh is not an expected part of a
+ * larger transaction, then we don't allow a
+ * schema reload during a transaction. This
+ * stops others from modifying our schema
+ * behind our backs
+ */
+ if (ldb_get_opaque(ldb,
+ "dsdb_schema_refresh_expected")
+ != (void *)1) {
+ return schema;
+ }
}
}
@@ -229,19 +238,9 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
* continue to hit the database to get the highest USN.
*/
- if (private_data->in_read_transaction > 0) {
- /*
- * We must give a static value of the metadata sequence number
- * during a read lock, otherwise, we will fail to load the
- * schema at runtime.
- */
- schema_seq_num = private_data->schema_seq_num_read_lock;
- ret = LDB_SUCCESS;
- } else {
- ret = schema_metadata_get_uint64(private_data,
- DSDB_METADATA_SCHEMA_SEQ_NUM,
- &schema_seq_num, 0);
- }
+ ret = schema_metadata_get_uint64(private_data,
+ DSDB_METADATA_SCHEMA_SEQ_NUM,
+ &schema_seq_num, 0);
if (schema != NULL) {
if (ret == LDB_SUCCESS) {
@@ -616,32 +615,26 @@ static int schema_read_lock(struct ldb_module *module)
{
struct schema_load_private_data *private_data =
talloc_get_type(ldb_module_get_private(module), struct schema_load_private_data);
- uint64_t schema_seq_num = 0;
+ int ret;
if (private_data == NULL) {
return ldb_next_read_lock(module);
}
- private_data->in_read_transaction++;
+ ret = ldb_next_read_lock(module);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
if (private_data->in_transaction == 0 &&
- private_data->in_read_transaction == 1) {
- /*
- * This appears to fail during the init path, so do not bother
- * checking the return, and return 0 (reload schema).
- */
- schema_metadata_get_uint64(private_data,
- DSDB_METADATA_SCHEMA_SEQ_NUM,
- &schema_seq_num, 0);
-
- private_data->schema_seq_num_read_lock = schema_seq_num;
-
+ private_data->in_read_transaction == 0) {
/* Try the schema refresh now */
dsdb_get_schema(ldb_module_get_ctx(module), NULL);
}
- return ldb_next_read_lock(module);
+ private_data->in_read_transaction++;
+ return LDB_SUCCESS;
}
static int schema_read_unlock(struct ldb_module *module)
@@ -653,10 +646,6 @@ static int schema_read_unlock(struct ldb_module *module)
return ldb_next_read_unlock(module);
}
- if (private_data->in_transaction == 0 &&
- private_data->in_read_transaction == 1) {
- private_data->schema_seq_num_read_lock = 0;
- }
private_data->in_read_transaction--;
return ldb_next_read_unlock(module);
--
2.11.0
From 6530ba67f293ee1677c7576b5af51230a17e0cfb Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 9 Apr 2018 18:13:59 +1200
Subject: [PATCH 14/17] dsdb: ensure we take out a read lock during the
dsdb_init
We have to also take it out in the partitions code when we load the
partition backends.
This ensures that the init handlers hold a whole-db lock just as the
search code does.
To ensure the locking count in schema_load is balanced, the
private data is now created in the first lock_read() call.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/partition.c | 24 +++++++++++++++++++++
source4/dsdb/samdb/ldb_modules/partition_init.c | 15 +++----------
source4/dsdb/samdb/ldb_modules/samba_dsdb.c | 17 +++++++++++++--
source4/dsdb/samdb/ldb_modules/schema_load.c | 28 ++++++++++++-------------
4 files changed, 55 insertions(+), 29 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 37e714d6e1b..9fb1b9dee12 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -1236,6 +1236,30 @@ int partition_read_lock(struct ldb_module *module)
* ordering
*/
+ if (data == NULL) {
+ TALLOC_CTX *mem_ctx = talloc_new(module);
+
+ data = talloc_zero(mem_ctx, struct partition_private_data);
+ if (data == NULL) {
+ talloc_free(mem_ctx);
+ return ldb_operr(ldb);
+ }
+
+ /*
+ * When used from Samba4, this message is set by the
+ * samba4 module, as a fixed value not read from the
+ * DB. This avoids listing modules in the DB
+ */
+ data->forced_module_msg = talloc_get_type(
+ ldb_get_opaque(ldb,
+ DSDB_OPAQUE_PARTITION_MODULE_MSG_OPAQUE_NAME),
+ struct ldb_message);
+
+ ldb_module_set_private(module, talloc_steal(module,
+ data));
+ talloc_free(mem_ctx);
+ }
+
/*
* This will lock the metadata partition (sam.ldb) and
* will also call event loops, so we do it before we
diff --git a/source4/dsdb/samdb/ldb_modules/partition_init.c b/source4/dsdb/samdb/ldb_modules/partition_init.c
index 9a6ac0c05a9..9a8bb7e211d 100644
--- a/source4/dsdb/samdb/ldb_modules/partition_init.c
+++ b/source4/dsdb/samdb/ldb_modules/partition_init.c
@@ -863,18 +863,9 @@ int partition_init(struct ldb_module *module)
return ldb_operr(ldb);
}
- data = talloc_zero(mem_ctx, struct partition_private_data);
- if (data == NULL) {
- return ldb_operr(ldb);
- }
-
- /* When used from Samba4, this message is set by the samba4
- * module, as a fixed value not read from the DB. This avoids
- * listing modules in the DB */
- data->forced_module_msg = talloc_get_type(
- ldb_get_opaque(ldb,
- DSDB_OPAQUE_PARTITION_MODULE_MSG_OPAQUE_NAME),
- struct ldb_message);
+ /* We actually got this during the read_lock call */
+ data = talloc_get_type_abort(ldb_module_get_private(module),
+ struct partition_private_data);
/* This loads the partitions */
ret = partition_reload_if_required(module, data, NULL);
diff --git a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
index 2605c1e511e..54ec6a26068 100644
--- a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
+++ b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
@@ -249,7 +249,7 @@ static bool check_required_features(struct ldb_message_element *el)
static int samba_dsdb_init(struct ldb_module *module)
{
struct ldb_context *ldb = ldb_module_get_ctx(module);
- int ret, len, i, j;
+ int ret, lock_ret, len, i, j;
TALLOC_CTX *tmp_ctx = talloc_new(module);
struct ldb_result *res;
struct ldb_message *rootdse_msg = NULL, *partition_msg;
@@ -627,7 +627,20 @@ static int samba_dsdb_init(struct ldb_module *module)
/* Set this as the 'next' module, so that we effectivly append it to module chain */
ldb_module_set_next(module, module_chain);
- return ldb_next_init(module);
+ ret = ldb_next_read_lock(module);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ ret = ldb_next_init(module);
+
+ lock_ret = ldb_next_read_unlock(module);
+
+ if (lock_ret != LDB_SUCCESS) {
+ return lock_ret;
+ }
+
+ return ret;
}
static const struct ldb_module_ops ldb_samba_dsdb_module_ops = {
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index cfd88fec467..e977e8b0ad5 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -498,17 +498,11 @@ static int schema_load(struct ldb_context *ldb,
static int schema_load_init(struct ldb_module *module)
{
struct ldb_context *ldb = ldb_module_get_ctx(module);
- struct schema_load_private_data *private_data;
+ struct schema_load_private_data *private_data =
+ talloc_get_type_abort(ldb_module_get_private(module),
+ struct schema_load_private_data);
int ret;
- private_data = talloc_zero(module, struct schema_load_private_data);
- if (private_data == NULL) {
- return ldb_oom(ldb);
- }
- private_data->module = module;
-
- ldb_module_set_private(module, private_data);
-
ret = ldb_next_init(module);
if (ret != LDB_SUCCESS) {
return ret;
@@ -618,7 +612,14 @@ static int schema_read_lock(struct ldb_module *module)
int ret;
if (private_data == NULL) {
- return ldb_next_read_lock(module);
+ private_data = talloc_zero(module, struct schema_load_private_data);
+ if (private_data == NULL) {
+ return ldb_module_oom(module);
+ }
+
+ private_data->module = module;
+
+ ldb_module_set_private(module, private_data);
}
ret = ldb_next_read_lock(module);
@@ -640,11 +641,8 @@ static int schema_read_lock(struct ldb_module *module)
static int schema_read_unlock(struct ldb_module *module)
{
struct schema_load_private_data *private_data =
- talloc_get_type(ldb_module_get_private(module), struct schema_load_private_data);
-
- if (private_data == NULL) {
- return ldb_next_read_unlock(module);
- }
+ talloc_get_type_abort(ldb_module_get_private(module),
+ struct schema_load_private_data);
private_data->in_read_transaction--;
--
2.11.0
From 689a464e0f24c92947d4ba94ba21719c16faf322 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 10 Apr 2018 13:34:56 +1200
Subject: [PATCH 15/17] dsdb: Use talloc_get_type_abort() in
schema_load_{start,end}_transaction
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13379
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/schema_load.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index e977e8b0ad5..7cbbeb699ee 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -514,7 +514,8 @@ static int schema_load_init(struct ldb_module *module)
static int schema_load_start_transaction(struct ldb_module *module)
{
struct schema_load_private_data *private_data =
- talloc_get_type(ldb_module_get_private(module), struct schema_load_private_data);
+ talloc_get_type_abort(ldb_module_get_private(module),
+ struct schema_load_private_data);
struct ldb_context *ldb = ldb_module_get_ctx(module);
struct dsdb_schema *schema;
int ret;
@@ -547,7 +548,8 @@ static int schema_load_start_transaction(struct ldb_module *module)
static int schema_load_end_transaction(struct ldb_module *module)
{
struct schema_load_private_data *private_data =
- talloc_get_type(ldb_module_get_private(module), struct schema_load_private_data);
+ talloc_get_type_abort(ldb_module_get_private(module),
+ struct schema_load_private_data);
struct ldb_context *ldb = ldb_module_get_ctx(module);
if (private_data->in_transaction == 0) {
--
2.11.0
From d992f59d82766871200d671f2fd47252b95ba6fa Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Mar 2018 13:44:52 +1300
Subject: [PATCH 16/17] ldb_wrap: Remove the magic cache of database handles
except for sam.ldb
sam.ldb is handled in samdb_connect_url(), not this function.
This cache caused issues when "private dir" was changed in a testing script, but also
just generates many-owner shared mutable state that is frowned upon these days.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb-samba/ldb_wrap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/ldb-samba/ldb_wrap.c b/lib/ldb-samba/ldb_wrap.c
index 8c3bf6f7bf3..53255556e7c 100644
--- a/lib/ldb-samba/ldb_wrap.c
+++ b/lib/ldb-samba/ldb_wrap.c
@@ -303,9 +303,13 @@ int samba_ldb_connect(struct ldb_context *ldb, struct loadparm_context *lp_ctx,
struct ldb_context *ldb;
int ret;
- ldb = ldb_wrap_find(url, ev, lp_ctx, session_info, credentials, flags);
- if (ldb != NULL)
- return talloc_reference(mem_ctx, ldb);
+ /*
+ * Unlike samdb_connect_url() do not try and cache the LDB
+ * handle, get a new one each time. Only sam.ldb is
+ * punitively expensive to open and helpful caches like this
+ * cause challenges (such as if the value for 'private dir'
+ * changes).
+ */
ldb = samba_ldb_init(mem_ctx, ev, lp_ctx, session_info, credentials);
@@ -318,11 +322,6 @@ int samba_ldb_connect(struct ldb_context *ldb, struct loadparm_context *lp_ctx,
return NULL;
}
- if (!ldb_wrap_add(url, ev, lp_ctx, session_info, credentials, flags, ldb)) {
- talloc_free(ldb);
- return NULL;
- }
-
DEBUG(3,("ldb_wrap open of %s\n", url));
return ldb;
--
2.11.0
From cb607346d3c7c662343b0eae69e43eaa6358c188 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 13 Mar 2018 16:43:54 +1300
Subject: [PATCH 17/17] ldb-samba: require pid match for cached ldb
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb-samba/ldb_wrap.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/ldb-samba/ldb_wrap.c b/lib/ldb-samba/ldb_wrap.c
index 53255556e7c..143e128d5d7 100644
--- a/lib/ldb-samba/ldb_wrap.c
+++ b/lib/ldb-samba/ldb_wrap.c
@@ -94,6 +94,8 @@ static struct ldb_wrap {
/* the context is what we use to tell if two ldb
* connections are exactly equivalent
*/
+ pid_t pid; /* We want to re-open in a new PID due to
+ * the LMDB backend */
const char *url;
struct tevent_context *ev;
struct loadparm_context *lp_ctx;
@@ -186,10 +188,12 @@ char *wrap_casefold(void *context, void *mem_ctx, const char *s, size_t n)
struct cli_credentials *credentials,
unsigned int flags)
{
+ pid_t pid = getpid();
struct ldb_wrap *w;
/* see if we can re-use an existing ldb */
for (w=ldb_wrap_list; w; w=w->next) {
- if (w->context.ev == ev &&
+ if (w->context.pid == pid &&
+ w->context.ev == ev &&
w->context.lp_ctx == lp_ctx &&
w->context.session_info == session_info &&
w->context.credentials == credentials &&
@@ -249,6 +253,7 @@ int samba_ldb_connect(struct ldb_context *ldb, struct loadparm_context *lp_ctx,
return false;
}
+ c.pid = getpid();
c.url = url;
c.ev = ev;
c.lp_ctx = lp_ctx;
--
2.11.0
More information about the samba-technical
mailing list