[WIP][PATCH] Safe LDB locking for better replication

Andrew Bartlett abartlet at samba.org
Thu Jun 15 04:58:24 UTC 2017


On Wed, 2017-06-14 at 22:02 +1200, Andrew Bartlett via samba-technical
wrote:
> > Further comments most welcome, and I plan to re-write it to use
> > read_lock() and read_unlock() operations tomorrow. 

Attached is the patch set against master

As mentioned previously, my TODO list is:
 - fix tests not to hang when failing
 - add test for the talloc_free(req) unlock case
 - add some kind of test for the partitions locking 
 - a dbcheck rule for out of date @ATTRIBUTES and @INDEXLIST (rather
than a runtime check)

Once I get this in, perhaps just the core set, I hope to get:
 - multi-process LDAP server
 - tests for the multi-process LDAP to show it honours the read and
transaction lock
 - Douglas's tdb multi-value work (in Garming's autobuild)
 - perf work

It is going to be a very busy two weeks.

If you could give your new feedback on this patch I would most
appreciate it.  However, given the timeframe before 4.7 I don't want to
do a major rework, this approach has proven stable under many, many
test runs and while perhaps not optimally efficient, we can improve
that later if need be. 

Specifically, given the things our LDB stack currently does, I don't
want to ban recursive (nested) search locks quite yet. 

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 1d2cc17c1ba4f065890f42364c620369cbea6f81 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 8 Jun 2017 23:05:26 +1200
Subject: [PATCH 01/18] dsdb: Rework schema_init module to avoid database write
 and use valid memory

The schema can go away unless the second argument (the memory context) is supplied

There is no need to write the @ATTRIBUTES and @INDEXLIST on every DB load
we only need to write it if the schema is changed, and the repl_meta_data module
will notice if that happens and trigger the DSDB_EXTENDED_SCHEMA_UPDATE_NOW_OID
extended operation.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/schema_load.c | 64 ++++++++++++++++++----------
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 6ffa4650dd9..3fba97e4f20 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -358,29 +358,15 @@ failed:
 	return ret;
 }	
 
-
-static int schema_load_init(struct ldb_module *module)
+static int schema_load(struct ldb_context *ldb,
+		       struct ldb_module *module)
 {
-	struct schema_load_private_data *private_data;
-	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	struct dsdb_schema *schema;
 	void *readOnlySchema;
 	int ret, metadata_ret;
-
-	private_data = talloc_zero(module, struct schema_load_private_data);
-	if (private_data == NULL) {
-		return ldb_oom(ldb);
-	}
-	private_data->module = module;
+	TALLOC_CTX *frame = talloc_stackframe();
 	
-	ldb_module_set_private(module, private_data);
-
-	ret = ldb_next_init(module);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
-	schema = dsdb_get_schema(ldb, NULL);
+	schema = dsdb_get_schema(ldb, frame);
 
 	metadata_ret = schema_metadata_open(module);
 
@@ -394,10 +380,12 @@ static int schema_load_init(struct ldb_module *module)
 				ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 					      "schema_load_init: dsdb_set_schema_refresh_fns() failed: %d:%s: %s",
 					      ret, ldb_strerror(ret), ldb_errstring(ldb));
+				TALLOC_FREE(frame);
 				return ret;
 			}
 		}
 
+		TALLOC_FREE(frame);
 		return LDB_SUCCESS;
 	}
 
@@ -408,11 +396,12 @@ static int schema_load_init(struct ldb_module *module)
 	 * have to update the backend server schema too */
 	if (readOnlySchema != NULL) {
 		struct dsdb_schema *new_schema;
-		ret = dsdb_schema_from_db(module, private_data, 0, &new_schema);
+		ret = dsdb_schema_from_db(module, frame, 0, &new_schema);
 		if (ret != LDB_SUCCESS) {
 			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 				      "schema_load_init: dsdb_schema_from_db() failed: %d:%s: %s",
 				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			TALLOC_FREE(frame);
 			return ret;
 		}
 
@@ -422,6 +411,7 @@ static int schema_load_init(struct ldb_module *module)
 			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 				      "schema_load_init: dsdb_set_schema() failed: %d:%s: %s",
 				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			TALLOC_FREE(frame);
 			return ret;
 		}
 
@@ -432,26 +422,54 @@ static int schema_load_init(struct ldb_module *module)
 			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 				      "schema_load_init: dsdb_set_schema_refresh_fns() failed: %d:%s: %s",
 				      ret, ldb_strerror(ret), ldb_errstring(ldb));
+			TALLOC_FREE(frame);
 			return ret;
 		}
 	} else {
 		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 			      "schema_load_init: failed to open metadata.tdb");
+		TALLOC_FREE(frame);
 		return metadata_ret;
 	}
 
-	schema = dsdb_get_schema(ldb, NULL);
+	schema = dsdb_get_schema(ldb, frame);
 
 	/* We do this, invoking the refresh handler, so we know that it works */
 	if (schema == NULL) {
 		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 			      "schema_load_init: dsdb_get_schema failed");
+		TALLOC_FREE(frame);
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	/* Now check the @INDEXLIST is correct, or fix it up */
-	ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
-						     true);
+	TALLOC_FREE(frame);
+	return LDB_SUCCESS;
+}
+
+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;
+
+	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;
+	}
+
+	ret = schema_load(ldb, module);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
 	if (ret != LDB_SUCCESS) {
 		ldb_asprintf_errstring(ldb, "Failed to update "
 				       "@INDEXLIST and @ATTRIBUTES "
-- 
2.11.0


From 3611c11614fbe08db7e7f6ff24becbf0c0e69a1d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 8 Jun 2017 23:17:20 +1200
Subject: [PATCH 02/18] dsdb: Do not prevent searches for @ATTRIBUTES because
 the DB is not set up yet

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/show_deleted.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/show_deleted.c b/source4/dsdb/samdb/ldb_modules/show_deleted.c
index 773dcfbf3fb..6b5fdaaa2c0 100644
--- a/source4/dsdb/samdb/ldb_modules/show_deleted.c
+++ b/source4/dsdb/samdb/ldb_modules/show_deleted.c
@@ -51,6 +51,11 @@ static int show_deleted_search(struct ldb_module *module, struct ldb_request *re
 	int ret;
 	const char *attr_filter = NULL;
 
+	/* do not manipulate our control entries */
+	if (ldb_dn_is_special(req->op.search.base)) {
+		return ldb_next_request(module, req);
+	}
+
 	ldb = ldb_module_get_ctx(module);
 
 	state = talloc_get_type(ldb_module_get_private(module), struct show_deleted_state);
-- 
2.11.0


From b0ac088ca5f1cac6ab660f19627d7363eaa41d28 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 31 Mar 2017 17:34:13 +1300
Subject: [PATCH 03/18] tdb: Remove locking from tdb_traverse_read()

This restores the original intent of tdb_traverse_read() in
7dd31288a701d772e45b1960ac4ce4cc1be782ed

This is needed to avoid a deadlock with tdb_lockall() and the
transaction start, as ldb_tdb should take the allrecord lock during a
search (which calls tdb_traverse), and can otherwise deadlock against
a transaction starting in another process

We add a test to show that a transaction can now start while a read
traverse is in progress

This allows more operations to happen in parallel.  The blocking point
is moved to the prepare commit.

This in turn permits a roughly doubling of unindexed search
performance, because currently ldb_tdb omits to take the lock due to
an unrelated bug, but taking the allrecord lock triggers the
above-mentioned deadlock.

This behaviour was added in 251aaafe3a9213118ac3a92def9ab2104c40d12a for
Solaris 10 in 2005. But the run-fcntl-deadlock test works also on Solaris 10,
see https://lists.samba.org/archive/samba-technical/2017-April/119876.html.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/tdb/common/traverse.c          | 10 +---------
 lib/tdb/test/run-nested-traverse.c | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/lib/tdb/common/traverse.c b/lib/tdb/common/traverse.c
index f33ef34ab8d..f62306e5560 100644
--- a/lib/tdb/common/traverse.c
+++ b/lib/tdb/common/traverse.c
@@ -244,7 +244,7 @@ out:
 
 
 /*
-  a read style traverse - temporarily marks the db read only
+  a read style traverse - temporarily marks each record read only
 */
 _PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb,
 		      tdb_traverse_func fn, void *private_data)
@@ -252,19 +252,11 @@ _PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb,
 	struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK };
 	int ret;
 
-	/* we need to get a read lock on the transaction lock here to
-	   cope with the lock ordering semantics of solaris10 */
-	if (tdb_transaction_lock(tdb, F_RDLCK, TDB_LOCK_WAIT)) {
-		return -1;
-	}
-
 	tdb->traverse_read++;
 	tdb_trace(tdb, "tdb_traverse_read_start");
 	ret = tdb_traverse_internal(tdb, fn, private_data, &tl);
 	tdb->traverse_read--;
 
-	tdb_transaction_unlock(tdb, F_RDLCK);
-
 	return ret;
 }
 
diff --git a/lib/tdb/test/run-nested-traverse.c b/lib/tdb/test/run-nested-traverse.c
index 22ee3e2a2a6..aeaa0859793 100644
--- a/lib/tdb/test/run-nested-traverse.c
+++ b/lib/tdb/test/run-nested-traverse.c
@@ -41,7 +41,30 @@ static int traverse2(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
 	return 0;
 }
 
-static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+static int traverse1r(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+		     void *p)
+{
+	ok1(correct_key(key));
+	ok1(correct_data(data));
+	ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
+	    == SUCCESS);
+	ok1(external_agent_operation(agent, STORE, tdb_name(tdb))
+	    == SUCCESS);
+	ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb))
+	    == WOULD_HAVE_BLOCKED);
+	tdb_traverse(tdb, traverse2, NULL);
+
+	/* That should *not* release the all-records lock! */
+	ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
+	    == SUCCESS);
+	ok1(external_agent_operation(agent, STORE, tdb_name(tdb))
+	    == SUCCESS);
+	ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb))
+	    == WOULD_HAVE_BLOCKED);
+	return 0;
+}
+
+static int traverse1w(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
 		     void *p)
 {
 	ok1(correct_key(key));
@@ -50,7 +73,7 @@ static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
 	    == WOULD_HAVE_BLOCKED);
 	tdb_traverse(tdb, traverse2, NULL);
 
-	/* That should *not* release the transaction lock! */
+	/* That should *not* release the all-records lock! */
 	ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb))
 	    == WOULD_HAVE_BLOCKED);
 	return 0;
@@ -80,8 +103,8 @@ int main(int argc, char *argv[])
 	data.dsize = strlen("world");
 
 	ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0);
-	tdb_traverse(tdb, traverse1, NULL);
-	tdb_traverse_read(tdb, traverse1, NULL);
+	tdb_traverse(tdb, traverse1w, NULL);
+	tdb_traverse_read(tdb, traverse1r, NULL);
 	tdb_close(tdb);
 
 	return exit_status();
-- 
2.11.0


From 9c6843ee286d872c9ef7b6ea9e30211c92b5750d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 11 Apr 2017 17:27:33 +0200
Subject: [PATCH 04/18] TODO: tdb: version 1.3.14

* allow tdb_traverse_read before tdb_transaction[_prepare]_commit()
---
 lib/tdb/ABI/tdb-1.3.14.sigs | 70 +++++++++++++++++++++++++++++++++++++++++++++
 lib/tdb/wscript             |  2 +-
 2 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 lib/tdb/ABI/tdb-1.3.14.sigs

diff --git a/lib/tdb/ABI/tdb-1.3.14.sigs b/lib/tdb/ABI/tdb-1.3.14.sigs
new file mode 100644
index 00000000000..48f4278890a
--- /dev/null
+++ b/lib/tdb/ABI/tdb-1.3.14.sigs
@@ -0,0 +1,70 @@
+tdb_add_flags: void (struct tdb_context *, unsigned int)
+tdb_append: int (struct tdb_context *, TDB_DATA, TDB_DATA)
+tdb_chainlock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_mark: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_nonblock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_read: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_read_nonblock: int (struct tdb_context *, TDB_DATA)
+tdb_chainlock_unmark: int (struct tdb_context *, TDB_DATA)
+tdb_chainunlock: int (struct tdb_context *, TDB_DATA)
+tdb_chainunlock_read: int (struct tdb_context *, TDB_DATA)
+tdb_check: int (struct tdb_context *, int (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_close: int (struct tdb_context *)
+tdb_delete: int (struct tdb_context *, TDB_DATA)
+tdb_dump_all: void (struct tdb_context *)
+tdb_enable_seqnum: void (struct tdb_context *)
+tdb_error: enum TDB_ERROR (struct tdb_context *)
+tdb_errorstr: const char *(struct tdb_context *)
+tdb_exists: int (struct tdb_context *, TDB_DATA)
+tdb_fd: int (struct tdb_context *)
+tdb_fetch: TDB_DATA (struct tdb_context *, TDB_DATA)
+tdb_firstkey: TDB_DATA (struct tdb_context *)
+tdb_freelist_size: int (struct tdb_context *)
+tdb_get_flags: int (struct tdb_context *)
+tdb_get_logging_private: void *(struct tdb_context *)
+tdb_get_seqnum: int (struct tdb_context *)
+tdb_hash_size: int (struct tdb_context *)
+tdb_increment_seqnum_nonblock: void (struct tdb_context *)
+tdb_jenkins_hash: unsigned int (TDB_DATA *)
+tdb_lock_nonblock: int (struct tdb_context *, int, int)
+tdb_lockall: int (struct tdb_context *)
+tdb_lockall_mark: int (struct tdb_context *)
+tdb_lockall_nonblock: int (struct tdb_context *)
+tdb_lockall_read: int (struct tdb_context *)
+tdb_lockall_read_nonblock: int (struct tdb_context *)
+tdb_lockall_unmark: int (struct tdb_context *)
+tdb_log_fn: tdb_log_func (struct tdb_context *)
+tdb_map_size: size_t (struct tdb_context *)
+tdb_name: const char *(struct tdb_context *)
+tdb_nextkey: TDB_DATA (struct tdb_context *, TDB_DATA)
+tdb_null: dptr = 0xXXXX, dsize = 0
+tdb_open: struct tdb_context *(const char *, int, int, int, mode_t)
+tdb_open_ex: struct tdb_context *(const char *, int, int, int, mode_t, const struct tdb_logging_context *, tdb_hash_func)
+tdb_parse_record: int (struct tdb_context *, TDB_DATA, int (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_printfreelist: int (struct tdb_context *)
+tdb_remove_flags: void (struct tdb_context *, unsigned int)
+tdb_reopen: int (struct tdb_context *)
+tdb_reopen_all: int (int)
+tdb_repack: int (struct tdb_context *)
+tdb_rescue: int (struct tdb_context *, void (*)(TDB_DATA, TDB_DATA, void *), void *)
+tdb_runtime_check_for_robust_mutexes: bool (void)
+tdb_set_logging_function: void (struct tdb_context *, const struct tdb_logging_context *)
+tdb_set_max_dead: void (struct tdb_context *, int)
+tdb_setalarm_sigptr: void (struct tdb_context *, volatile sig_atomic_t *)
+tdb_store: int (struct tdb_context *, TDB_DATA, TDB_DATA, int)
+tdb_storev: int (struct tdb_context *, TDB_DATA, const TDB_DATA *, int, int)
+tdb_summary: char *(struct tdb_context *)
+tdb_transaction_cancel: int (struct tdb_context *)
+tdb_transaction_commit: int (struct tdb_context *)
+tdb_transaction_prepare_commit: int (struct tdb_context *)
+tdb_transaction_start: int (struct tdb_context *)
+tdb_transaction_start_nonblock: int (struct tdb_context *)
+tdb_transaction_write_lock_mark: int (struct tdb_context *)
+tdb_transaction_write_lock_unmark: int (struct tdb_context *)
+tdb_traverse: int (struct tdb_context *, tdb_traverse_func, void *)
+tdb_traverse_read: int (struct tdb_context *, tdb_traverse_func, void *)
+tdb_unlock: int (struct tdb_context *, int, int)
+tdb_unlockall: int (struct tdb_context *)
+tdb_unlockall_read: int (struct tdb_context *)
+tdb_validate_freelist: int (struct tdb_context *, int *)
+tdb_wipe_all: int (struct tdb_context *)
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 09bc0a3192c..478255038c7 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'tdb'
-VERSION = '1.3.13'
+VERSION = '1.3.14'
 
 blddir = 'bin'
 
-- 
2.11.0


From 78a1745e233b2aeed6f82197edf37f2b34aa01e1 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 30 Mar 2017 12:03:17 +1300
Subject: [PATCH 05/18] ldb_tdb: Ensure we correctly decrement
 ltdb->read_lock_count

If we do not do this, then we never take the all record lock, and instead do a lock
for every record as we go, which is very slow during a large search

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 261011eb99c..5c6d3f5544b 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -119,6 +119,7 @@ int ltdb_unlock_read(struct ldb_module *module)
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 	if (ltdb->in_transaction == 0 && ltdb->read_lock_count == 1) {
 		tdb_unlockall_read(ltdb->tdb);
+		ltdb->read_lock_count--;
 		return 0;
 	}
 	ltdb->read_lock_count--;
-- 
2.11.0


From 6aaec290044f687229060f3fe19d2ee53e5a5ce1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 25 Apr 2017 22:33:53 +1200
Subject: [PATCH 06/18] ldb: Show that writes do not appear during an
 ldb_search()

A modify or rename during a search must not cause a search to change
output, and attributes having an index should in particular not see
any change in behaviour in this respect

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tests/ldb_mod_op_test.c | 340 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 340 insertions(+)

diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 9bcbb2fc76d..0f4d9c6e8bd 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1507,6 +1507,334 @@ static void test_ldb_search_against_transaction(void **state)
 
 }
 
+/*
+ * This test is also complex.
+ * The purpose is to test if a modify can occour during a ldb_search()
+ * This would be a failure if if in process
+ * (1) and (2):
+ *  - (1) ltdb_search() starts and calls back for one entry
+ *  - (2) one of the entries to be matched is modified
+ *  - (1) the indexed search tries to return the modified entry, but
+ *        it is no longer found, either:
+ *          - despite it still matching (dn changed)
+ *          - it no longer matching (attrs changed)
+ *
+ * We also try un-indexed to show that the behaviour differs on this
+ * point, which it should not (an index should only impact search
+ * speed).
+ */
+
+struct modify_during_search_test_ctx {
+	struct ldbtest_ctx *test_ctx;
+	int res_count;
+	pid_t child_pid;
+	struct ldb_dn *basedn;
+	bool got_cn;
+	bool got_2_cn;
+	bool rename;
+};
+
+/*
+ * This purpose of this callback is to trigger a write in
+ * the child process while a search is in progress.
+ *
+ * In tdb 1.3.12 tdb_traverse_read() take the read transaction lock
+ * however in ldb 1.1.29 ltdb_search() forgets to take the all-record
+ * lock (except the very first time) due to a ref-counting bug.
+ *
+ * We assume that if the write will proceed, it will proceed in a 3
+ * second window after the function is called.
+ */
+
+static int test_ldb_modify_during_search_callback1(struct ldb_request *req,
+						   struct ldb_reply *ares)
+{
+	int ret;
+	int pipes[2];
+	char buf[2];
+	struct modify_during_search_test_ctx *ctx = req->context;
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+	{
+		const struct ldb_val *cn_val
+			= ldb_dn_get_component_val(ares->message->dn, 0);
+		const char *cn = (char *)cn_val->data;
+		ctx->res_count++;
+		if (strcmp(cn, "test_search_cn") == 0) {
+			ctx->got_cn = true;
+		} else if (strcmp(cn, "test_search_2_cn") == 0) {
+			ctx->got_2_cn = true;
+		}
+		if (ctx->res_count == 2) {
+			return LDB_SUCCESS;
+		}
+		break;
+	}
+	case LDB_REPLY_REFERRAL:
+		return LDB_SUCCESS;
+
+	case LDB_REPLY_DONE:
+		return ldb_request_done(req, LDB_SUCCESS);
+	}
+
+	ret = pipe(pipes);
+	assert_int_equal(ret, 0);
+
+	ctx->child_pid = fork();
+	if (ctx->child_pid == 0 && ctx->rename) {
+		TALLOC_CTX *tmp_ctx = NULL;
+		struct ldb_dn *dn, *new_dn;
+		TALLOC_FREE(ctx->test_ctx->ldb);
+		TALLOC_FREE(ctx->test_ctx->ev);
+		ctx->test_ctx->ev = tevent_context_init(ctx->test_ctx);
+		if (ctx->test_ctx->ev == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ctx->test_ctx->ldb = ldb_init(ctx->test_ctx,
+					      ctx->test_ctx->ev);
+		if (ctx->test_ctx->ldb == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_connect(ctx->test_ctx->ldb,
+				  ctx->test_ctx->dbpath, 0, NULL);
+		if (ret != LDB_SUCCESS) {
+			exit(ret);
+		}
+
+		tmp_ctx = talloc_new(ctx->test_ctx);
+		if (tmp_ctx == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		if (ctx->got_cn) {
+			/* Modify the other one */
+			dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+					    "cn=test_search_2_cn,"
+					    "dc=search_test_entry");
+		} else {
+			dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+					    "cn=test_search_cn,"
+					    "dc=search_test_entry");
+		}
+		if (dn == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		new_dn = ldb_dn_new_fmt(tmp_ctx, ctx->test_ctx->ldb,
+					"cn=test_search_cn_renamed,"
+					"dc=search_test_entry");
+		if (new_dn == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_transaction_start(ctx->test_ctx->ldb);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		if (write(pipes[1], "GO", 2) != 2) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_rename(ctx->test_ctx->ldb, dn, new_dn);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		ret = ldb_transaction_commit(ctx->test_ctx->ldb);
+		exit(ret);
+
+	} else if (ctx->child_pid == 0) {
+		TALLOC_CTX *tmp_ctx = NULL;
+		struct ldb_message *msg;
+		struct ldb_message_element *el;
+		TALLOC_FREE(ctx->test_ctx->ldb);
+		TALLOC_FREE(ctx->test_ctx->ev);
+		ctx->test_ctx->ev = tevent_context_init(ctx->test_ctx);
+		if (ctx->test_ctx->ev == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ctx->test_ctx->ldb = ldb_init(ctx->test_ctx,
+					      ctx->test_ctx->ev);
+		if (ctx->test_ctx->ldb == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_connect(ctx->test_ctx->ldb,
+				  ctx->test_ctx->dbpath, 0, NULL);
+		if (ret != LDB_SUCCESS) {
+			exit(ret);
+		}
+
+		tmp_ctx = talloc_new(ctx->test_ctx);
+		if (tmp_ctx == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		msg = ldb_msg_new(tmp_ctx);
+		if (msg == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		if (ctx->got_cn) {
+			/* Modify the other one */
+			msg->dn = ldb_dn_new_fmt(msg, ctx->test_ctx->ldb,
+						 "cn=test_search_2_cn,"
+						 "dc=search_test_entry");
+		} else {
+			msg->dn = ldb_dn_new_fmt(msg, ctx->test_ctx->ldb,
+						 "cn=test_search_cn,"
+						 "dc=search_test_entry");
+		}
+		if (msg->dn == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_msg_add_string(msg, "filterAttr", "TRUE");
+		if (ret != 0) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+		el = ldb_msg_find_element(msg, "filterAttr");
+		if (el == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+		el->flags = LDB_FLAG_MOD_REPLACE;
+
+		ret = ldb_transaction_start(ctx->test_ctx->ldb);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		if (write(pipes[1], "GO", 2) != 2) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_modify(ctx->test_ctx->ldb, msg);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		ret = ldb_transaction_commit(ctx->test_ctx->ldb);
+		exit(ret);
+	}
+
+	ret = read(pipes[0], buf, 2);
+	assert_int_equal(ret, 2);
+
+	sleep(3);
+
+	return LDB_SUCCESS;
+}
+
+static void test_ldb_modify_during_search(void **state, bool add_index,
+					  bool rename)
+{
+	struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+			struct search_test_ctx);
+	struct modify_during_search_test_ctx
+		ctx =
+		{ .res_count = 0,
+		  .test_ctx = search_test_ctx->ldb_test_ctx,
+		  .rename = rename
+		};
+
+	int ret;
+	struct ldb_request *req;
+	pid_t pid;
+	int wstatus;
+
+	if (add_index) {
+		struct ldb_message *msg;
+		struct ldb_dn *indexlist = ldb_dn_new(search_test_ctx,
+						      search_test_ctx->ldb_test_ctx->ldb,
+						      "@INDEXLIST");
+		assert_non_null(indexlist);
+
+		msg = ldb_msg_new(search_test_ctx);
+		assert_non_null(msg);
+
+		msg->dn = indexlist;
+
+		ret = ldb_msg_add_string(msg, "@IDXATTR", "cn");
+		assert_int_equal(ret, LDB_SUCCESS);
+
+		ret = ldb_add(search_test_ctx->ldb_test_ctx->ldb,
+			      msg);
+
+		assert_int_equal(ret, LDB_SUCCESS);
+	}
+
+	tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
+
+	ctx.basedn
+		= ldb_dn_new_fmt(search_test_ctx,
+				 search_test_ctx->ldb_test_ctx->ldb,
+				 "%s",
+				 search_test_ctx->base_dn);
+	assert_non_null(ctx.basedn);
+
+
+	/*
+	 * This search must be over multiple items, and should include
+	 * the new name after a rename, to show that it would match
+	 * both before and after that modify
+	 */
+	ret = ldb_build_search_req(&req,
+				   search_test_ctx->ldb_test_ctx->ldb,
+				   search_test_ctx,
+				   ctx.basedn,
+				   LDB_SCOPE_SUBTREE,
+				   "(&(!(filterAttr=*))"
+				   "(|(cn=test_search_cn_renamed)(cn=test_search_cn)(cn=test_search_2_cn)))",
+				   NULL,
+				   NULL,
+				   &ctx,
+				   test_ldb_modify_during_search_callback1,
+				   NULL);
+	assert_int_equal(ret, 0);
+	ret = ldb_request(search_test_ctx->ldb_test_ctx->ldb, req);
+
+	if (ret == LDB_SUCCESS) {
+		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
+	}
+	assert_int_equal(ret, 0);
+	assert_int_equal(ctx.res_count, 2);
+	assert_int_equal(ctx.got_cn, true);
+	assert_int_equal(ctx.got_2_cn, true);
+
+	pid = waitpid(ctx.child_pid, &wstatus, 0);
+	assert_int_equal(pid, ctx.child_pid);
+
+	assert_true(WIFEXITED(wstatus));
+
+	assert_int_equal(WEXITSTATUS(wstatus), 0);
+
+
+}
+
+static void test_ldb_modify_during_indexed_search(void **state)
+{
+	return test_ldb_modify_during_search(state, true, false);
+}
+
+static void test_ldb_modify_during_unindexed_search(void **state)
+{
+	return test_ldb_modify_during_search(state, false, false);
+}
+
+static void test_ldb_rename_during_indexed_search(void **state)
+{
+	return test_ldb_modify_during_search(state, true, true);
+}
+
+static void test_ldb_rename_during_unindexed_search(void **state)
+{
+	return test_ldb_modify_during_search(state, false, true);
+}
+
 static int ldb_case_test_setup(void **state)
 {
 	int ret;
@@ -2056,6 +2384,18 @@ int main(int argc, const char **argv)
 		cmocka_unit_test_setup_teardown(test_ldb_search_against_transaction,
 						ldb_search_test_setup,
 						ldb_search_test_teardown),
+		cmocka_unit_test_setup_teardown(test_ldb_modify_during_unindexed_search,
+						ldb_search_test_setup,
+						ldb_search_test_teardown),
+		cmocka_unit_test_setup_teardown(test_ldb_modify_during_indexed_search,
+						ldb_search_test_setup,
+						ldb_search_test_teardown),
+		cmocka_unit_test_setup_teardown(test_ldb_rename_during_unindexed_search,
+						ldb_search_test_setup,
+						ldb_search_test_teardown),
+		cmocka_unit_test_setup_teardown(test_ldb_rename_during_indexed_search,
+						ldb_search_test_setup,
+						ldb_search_test_teardown),
 		cmocka_unit_test_setup_teardown(test_ldb_attrs_case_insensitive,
 						ldb_case_test_setup,
 						ldb_case_test_teardown),
-- 
2.11.0


From 866c9c29445bd7e9c85a1ac659a72ae47ac14544 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 22 May 2017 16:18:20 +1200
Subject: [PATCH 07/18] ldb: Add test encoding current locking behaviour during
 ldb_search()

Currently, a lock is not held against modifications once the final
record is returned via a callback, so modifications can be made
during the DONE callback.  This makes it hard to write modules
that interpert an ldb search result and do further processing
so will change in the future to allow the full search to be
atomic.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/tests/ldb_mod_op_test.c | 204 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 204 insertions(+)

diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 0f4d9c6e8bd..9ccb4cb6e75 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1835,6 +1835,207 @@ static void test_ldb_rename_during_unindexed_search(void **state)
 	return test_ldb_modify_during_search(state, false, true);
 }
 
+/*
+ * This test is also complex.
+ *
+ * The purpose is to test if a modify can occour during a ldb_search()
+ * before the end of the callback
+ *
+ * This would be a failure if if in process
+ * (1) and (2):
+ *  - (1) ldb_search() starts and calls back for a number of entries
+ *  - (2) an entry in the DB is allowed to change before the callback returns
+ *  - (1) the callback can see the modification
+ *
+ */
+
+/*
+ * This purpose of this callback is to trigger a write in
+ * the child process while a search DONE callback is in progress.
+ *
+ * In ldb 1.1.29 ldb_search() omitted to take a all-record
+ * lock for the full duration of the search and callbacks
+ *
+ * We assume that if the write will proceed, it will proceed in a 3
+ * second window after the function is called.
+ */
+
+static int test_ldb_modify_during_whole_search_callback1(struct ldb_request *req,
+							 struct ldb_reply *ares)
+{
+	int ret;
+	int pipes[2];
+	char buf[2];
+	struct modify_during_search_test_ctx *ctx = req->context;
+	struct ldb_dn *search_dn;
+	struct ldb_result *res2;
+
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+	case LDB_REPLY_REFERRAL:
+		return LDB_SUCCESS;
+
+	case LDB_REPLY_DONE:
+		break;
+	}
+
+	ret = pipe(pipes);
+	assert_int_equal(ret, 0);
+
+	ctx->child_pid = fork();
+	if (ctx->child_pid == 0) {
+		TALLOC_CTX *tmp_ctx = NULL;
+		struct ldb_message *msg;
+		struct ldb_message_element *el;
+		TALLOC_FREE(ctx->test_ctx->ldb);
+		TALLOC_FREE(ctx->test_ctx->ev);
+		ctx->test_ctx->ev = tevent_context_init(ctx->test_ctx);
+		if (ctx->test_ctx->ev == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ctx->test_ctx->ldb = ldb_init(ctx->test_ctx,
+					      ctx->test_ctx->ev);
+		if (ctx->test_ctx->ldb == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_connect(ctx->test_ctx->ldb,
+				  ctx->test_ctx->dbpath, 0, NULL);
+		if (ret != LDB_SUCCESS) {
+			exit(ret);
+		}
+
+		tmp_ctx = talloc_new(ctx->test_ctx);
+		if (tmp_ctx == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		msg = ldb_msg_new(tmp_ctx);
+		if (msg == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		msg->dn = ldb_dn_new_fmt(msg, ctx->test_ctx->ldb,
+					 "cn=test_search_cn,"
+					 "dc=search_test_entry");
+		if (msg->dn == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_msg_add_string(msg, "filterAttr", "TRUE");
+		if (ret != 0) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+		el = ldb_msg_find_element(msg, "filterAttr");
+		if (el == NULL) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+		el->flags = LDB_FLAG_MOD_REPLACE;
+
+		ret = ldb_transaction_start(ctx->test_ctx->ldb);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		if (write(pipes[1], "GO", 2) != 2) {
+			exit(LDB_ERR_OPERATIONS_ERROR);
+		}
+
+		ret = ldb_modify(ctx->test_ctx->ldb, msg);
+		if (ret != 0) {
+			exit(ret);
+		}
+
+		ret = ldb_transaction_commit(ctx->test_ctx->ldb);
+		exit(ret);
+	}
+
+	ret = read(pipes[0], buf, 2);
+	assert_int_equal(ret, 2);
+
+	sleep(3);
+
+	/*
+	 * If writes are not blocked until after this function, we
+	 * will be able to successfully search for this modification
+	 * here
+	 */
+
+	search_dn = ldb_dn_new_fmt(ares, ctx->test_ctx->ldb,
+				   "cn=test_search_cn,"
+				   "dc=search_test_entry");
+
+	ret = ldb_search(ctx->test_ctx->ldb, ares,
+			 &res2, search_dn, LDB_SCOPE_BASE, NULL,
+			 "filterAttr=TRUE");
+	assert_int_equal(ret, 0);
+
+	/* We got the result */
+	assert_int_equal(res2->count, 1);
+
+	return ldb_request_done(req, LDB_SUCCESS);
+}
+
+static void test_ldb_modify_during_whole_search(void **state)
+{
+	struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+			struct search_test_ctx);
+	struct modify_during_search_test_ctx
+		ctx =
+		{
+		  .test_ctx = search_test_ctx->ldb_test_ctx,
+		};
+
+	int ret;
+	struct ldb_request *req;
+	pid_t pid;
+	int wstatus;
+
+	tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
+
+	ctx.basedn
+		= ldb_dn_new_fmt(search_test_ctx,
+				 search_test_ctx->ldb_test_ctx->ldb,
+				 "%s",
+				 search_test_ctx->base_dn);
+	assert_non_null(ctx.basedn);
+
+
+	/*
+	 * The search just needs to call DONE, we don't care about the
+	 * contents of the search for this test
+	 */
+	ret = ldb_build_search_req(&req,
+				   search_test_ctx->ldb_test_ctx->ldb,
+				   search_test_ctx,
+				   ctx.basedn,
+				   LDB_SCOPE_SUBTREE,
+				   "(&(!(filterAttr=*))"
+				   "(cn=test_search_cn))",
+				   NULL,
+				   NULL,
+				   &ctx,
+				   test_ldb_modify_during_whole_search_callback1,
+				   NULL);
+	assert_int_equal(ret, 0);
+	ret = ldb_request(search_test_ctx->ldb_test_ctx->ldb, req);
+
+	if (ret == LDB_SUCCESS) {
+		ret = ldb_wait(req->handle, LDB_WAIT_ALL);
+	}
+	assert_int_equal(ret, 0);
+
+	pid = waitpid(ctx.child_pid, &wstatus, 0);
+	assert_int_equal(pid, ctx.child_pid);
+
+	assert_true(WIFEXITED(wstatus));
+
+	assert_int_equal(WEXITSTATUS(wstatus), 0);
+
+
+}
+
 static int ldb_case_test_setup(void **state)
 {
 	int ret;
@@ -2396,6 +2597,9 @@ int main(int argc, const char **argv)
 		cmocka_unit_test_setup_teardown(test_ldb_rename_during_indexed_search,
 						ldb_search_test_setup,
 						ldb_search_test_teardown),
+		cmocka_unit_test_setup_teardown(test_ldb_modify_during_whole_search,
+						ldb_search_test_setup,
+						ldb_search_test_teardown),
 		cmocka_unit_test_setup_teardown(test_ldb_attrs_case_insensitive,
 						ldb_case_test_setup,
 						ldb_case_test_teardown),
-- 
2.11.0


From 164cb058595b5a024635e7a4b3edb7656a77b2b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 12:10:51 +1200
Subject: [PATCH 08/18] ldb: Add read_lock and read_unlock to ldb_module_ops

This will be used to implement read locking in ldb_tdb

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/common/ldb_modules.c | 38 ++++++++++++++++++++++++++++++++++++++
 lib/ldb/include/ldb_module.h |  4 ++++
 2 files changed, 42 insertions(+)

diff --git a/lib/ldb/common/ldb_modules.c b/lib/ldb/common/ldb_modules.c
index ca93299ab00..f50b6c56130 100644
--- a/lib/ldb/common/ldb_modules.c
+++ b/lib/ldb/common/ldb_modules.c
@@ -641,6 +641,44 @@ int ldb_next_end_trans(struct ldb_module *module)
 	return ret;
 }
 
+int ldb_next_read_lock(struct ldb_module *module)
+{
+	int ret;
+	FIND_OP(module, read_lock);
+	ret = module->ops->read_lock(module);
+	if (ret == LDB_SUCCESS) {
+		return ret;
+	}
+	if (!ldb_errstring(module->ldb)) {
+		/* Set a default error string, to place the blame somewhere */
+		ldb_asprintf_errstring(module->ldb, "read_lock error in module %s: %s (%d)", module->ops->name, ldb_strerror(ret), ret);
+	}
+	if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+		ldb_debug(module->ldb, LDB_DEBUG_TRACE, "ldb_next_read_lock error: %s",
+			  ldb_errstring(module->ldb));
+	}
+	return ret;
+}
+
+int ldb_next_read_unlock(struct ldb_module *module)
+{
+	int ret;
+	FIND_OP(module, read_unlock);
+	ret = module->ops->read_unlock(module);
+	if (ret == LDB_SUCCESS) {
+		return ret;
+	}
+	if (!ldb_errstring(module->ldb)) {
+		/* Set a default error string, to place the blame somewhere */
+		ldb_asprintf_errstring(module->ldb, "read_unlock error in module %s: %s (%d)", module->ops->name, ldb_strerror(ret), ret);
+	}
+	if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+		ldb_debug(module->ldb, LDB_DEBUG_TRACE, "ldb_next_read_unlock error: %s",
+			  ldb_errstring(module->ldb));
+	}
+	return ret;
+}
+
 int ldb_next_prepare_commit(struct ldb_module *module)
 {
 	int ret;
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 3d56e68eddd..0f5feeb084c 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -77,6 +77,8 @@ struct ldb_module_ops {
 	int (*end_transaction)(struct ldb_module *);
 	int (*del_transaction)(struct ldb_module *);
 	int (*sequence_number)(struct ldb_module *, struct ldb_request *);
+	int (*read_lock)(struct ldb_module *);
+	int (*read_unlock)(struct ldb_module *);
 	void *private_data;
 };
 
@@ -203,6 +205,8 @@ int ldb_next_end_trans(struct ldb_module *module);
 int ldb_next_del_trans(struct ldb_module *module);
 int ldb_next_prepare_commit(struct ldb_module *module);
 int ldb_next_init(struct ldb_module *module);
+int ldb_next_read_lock(struct ldb_module *module);
+int ldb_next_read_unlock(struct ldb_module *module);
 
 void ldb_set_errstring(struct ldb_context *ldb, const char *err_string);
 void ldb_asprintf_errstring(struct ldb_context *ldb, const char *format, ...) PRINTF_ATTRIBUTE(2,3);
-- 
2.11.0


From b3afab1d495c9f3e4b6a5a18a93d5087a559b906 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 12 May 2017 01:39:08 +0200
Subject: [PATCH 09/18] ldb_tdb: Implement read_lock and read_unlock module
 operations

This allows Samba to provide a consistent view of the DB
despite the use of multiple databases via the partitions module
and over multiple callbacks via a module stack.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 5c6d3f5544b..1be954a7deb 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1543,6 +1543,8 @@ static const struct ldb_module_ops ltdb_ops = {
 	.end_transaction   = ltdb_end_trans,
 	.prepare_commit    = ltdb_prepare_commit,
 	.del_transaction   = ltdb_del_trans,
+	.read_lock         = ltdb_lock_read,
+	.read_unlock       = ltdb_unlock_read,
 };
 
 /*
-- 
2.11.0


From 6a3f758057970df2fbc3a19339423752667f88a8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 13:56:46 +1200
Subject: [PATCH 10/18] ldb: Lock the whole backend database for the duration
 of a search

We must hold locks not just for the duration of each search, but for the whole search
as our module stack may make multiple search requests to build up the whole result.

This is explains a number of replication and read corruption issues in Samba

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/common/ldb.c            | 158 +++++++++++++++++++++++++++++++++++++++-
 lib/ldb/tests/ldb_mod_op_test.c |  38 +++++++++-
 2 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index c9503303331..d296dace066 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -966,6 +966,145 @@ static int ldb_msg_check_element_flags(struct ldb_context *ldb,
 	return LDB_SUCCESS;
 }
 
+/*
+ * This context allows us to make the unlock be a talloc destructor
+ *
+ * This ensures that a request started, but not waited on, will still
+ * unlock.
+ */
+struct ldb_db_lock_context {
+	struct ldb_request *req;
+	struct ldb_context *ldb;
+};
+
+/*
+ * We have to have a the unlock on a destructor so that we unlock the
+ * DB if a caller calls talloc_free(req).  We trust that the ldb
+ * context has not already gone away.
+ */
+static int ldb_db_lock_destructor(struct ldb_db_lock_context *lock_context)
+{
+	int ret;
+	struct ldb_module *next_module;
+	FIRST_OP_NOERR(lock_context->ldb, read_unlock);
+	if (next_module != NULL) {
+		ret = next_module->ops->read_unlock(next_module);
+	} else {
+		ret = LDB_SUCCESS;
+	}
+
+	if (ret != LDB_SUCCESS) {
+		ldb_debug(lock_context->ldb,
+			  LDB_DEBUG_FATAL,
+			  "Failed to unlock db: %s / %s",
+			  ldb_errstring(lock_context->ldb),
+			  ldb_strerror(ret));
+	}
+	return 0;
+}
+
+static int ldb_lock_backend_callback(struct ldb_request *req,
+				     struct ldb_reply *ares)
+{
+	struct ldb_db_lock_context *lock_context;
+	int ret;
+
+	lock_context = talloc_get_type(req->context,
+				       struct ldb_db_lock_context);
+
+	if (!ares) {
+		return ldb_module_done(lock_context->req, NULL, NULL,
+					LDB_ERR_OPERATIONS_ERROR);
+	}
+	if (ares->error != LDB_SUCCESS || ares->type == LDB_REPLY_DONE) {
+		ret = ldb_module_done(lock_context->req, ares->controls,
+				      ares->response, ares->error);
+		/*
+		 * If this is a LDB_REPLY_DONE or an error, unlock the
+		 * DB by calling the destructor on this context
+		 */
+		talloc_free(lock_context);
+		return ret;
+	}
+
+	/* Otherwise pass on the callback */
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+		return ldb_module_send_entry(lock_context->req, ares->message, ares->controls);
+
+	case LDB_REPLY_REFERRAL:
+		return ldb_module_send_referral(lock_context->req, ares->referral);
+	default:
+		/* Can't happen */
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+}
+
+/*
+ * Do an ldb_search() with a lock held, but release it if the request
+ * is freed with talloc_free()
+ */
+static int lock_search(struct ldb_module *lock_module, struct ldb_request *req)
+{
+	/* Used in FIRST_OP_NOERR to find where to send the lock request */
+	struct ldb_module *next_module = NULL;
+
+	struct ldb_request *down_req = NULL;
+	struct ldb_db_lock_context *lock_context;
+	struct ldb_context *ldb = ldb_module_get_ctx(lock_module);
+	int ret;
+
+	lock_context = talloc(req, struct ldb_db_lock_context);
+	if (lock_context == NULL) {
+		return ldb_oom(ldb);
+	}
+
+	lock_context->ldb = ldb;
+	lock_context->req = req;
+
+	ret = ldb_build_search_req_ex(&down_req, ldb, req,
+				      req->op.search.base,
+				      req->op.search.scope,
+				      req->op.search.tree,
+				      req->op.search.attrs,
+				      req->controls,
+				      lock_context,
+				      ldb_lock_backend_callback,
+				      req);
+	LDB_REQ_SET_LOCATION(down_req);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	/* call DB lock */
+	FIRST_OP_NOERR(ldb, read_lock);
+	if (next_module != NULL) {
+		ret = next_module->ops->read_lock(next_module);
+	} else {
+		ret = LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION;
+	}
+
+	if (ret == LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION) {
+		/* We might be talking LDAP */
+		ldb_reset_err_string(ldb);
+		ret = 0;
+		TALLOC_FREE(lock_context);
+
+		return ldb_next_request(lock_module, req);
+	} else if ((ret != LDB_SUCCESS) && (ldb->err_string == NULL)) {
+		/* if no error string was setup by the backend */
+		ldb_asprintf_errstring(ldb, "Failed to get DB lock: %s (%d)",
+				       ldb_strerror(ret), ret);
+	} else {
+		talloc_set_destructor(lock_context, ldb_db_lock_destructor);
+	}
+
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	return ldb_next_request(lock_module, down_req);
+}
 
 /*
   start an ldb request
@@ -991,15 +1130,32 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
 	/* call the first module in the chain */
 	switch (req->operation) {
 	case LDB_SEARCH:
+	{
+		/*
+		 * A fake module to allow ldb_next_request() to be
+		 * re-used and to keep the locking out of this fucntion.
+		 */
+		struct ldb_module_ops lock_module_ops = {
+			.name = "lock_searches",
+			.search = lock_search
+		};
+		struct ldb_module lock_module = {
+			.ldb = ldb,
+			.next = ldb->modules,
+			.ops = &lock_module_ops
+		};
+		next_module = &lock_module;
+
 		/* due to "ldb_build_search_req" base DN always != NULL */
 		if (!ldb_dn_validate(req->op.search.base)) {
 			ldb_asprintf_errstring(ldb, "ldb_search: invalid basedn '%s'",
 					       ldb_dn_get_linearized(req->op.search.base));
 			return LDB_ERR_INVALID_DN_SYNTAX;
 		}
-		FIRST_OP(ldb, search);
+
 		ret = next_module->ops->search(next_module, req);
 		break;
+	}
 	case LDB_ADD:
 		if (!ldb_dn_validate(req->op.add.message->dn)) {
 			ldb_asprintf_errstring(ldb, "ldb_add: invalid dn '%s'",
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 9ccb4cb6e75..2fd92481257 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -1432,6 +1432,12 @@ static int test_ldb_search_against_transaction_callback1(struct ldb_request *req
 				   ctx,
 				   test_ldb_search_against_transaction_callback2,
 				   NULL);
+	/*
+	 * NOTE WELL:
+	 * If these assertions fail, we will also fail to
+	 * clean up as we hold locks
+	 */
+
 	assert_int_equal(ret, 0);
 	ret = ldb_request(ctx->test_ctx->ldb, req);
 
@@ -1969,10 +1975,16 @@ static int test_ldb_modify_during_whole_search_callback1(struct ldb_request *req
 	ret = ldb_search(ctx->test_ctx->ldb, ares,
 			 &res2, search_dn, LDB_SCOPE_BASE, NULL,
 			 "filterAttr=TRUE");
+
+	/*
+	 * NOTE WELL:
+	 * If these assertions fail, we will also fail to
+	 * clean up as we hold locks
+	 */
 	assert_int_equal(ret, 0);
 
-	/* We got the result */
-	assert_int_equal(res2->count, 1);
+	/* We should not have got the result */
+	assert_int_equal(res2->count, 0);
 
 	return ldb_request_done(req, LDB_SUCCESS);
 }
@@ -1991,6 +2003,8 @@ static void test_ldb_modify_during_whole_search(void **state)
 	struct ldb_request *req;
 	pid_t pid;
 	int wstatus;
+	struct ldb_dn *search_dn;
+	struct ldb_result *res2;
 
 	tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
 
@@ -2033,6 +2047,26 @@ static void test_ldb_modify_during_whole_search(void **state)
 
 	assert_int_equal(WEXITSTATUS(wstatus), 0);
 
+	/*
+	 * If writes are blocked until after the search function, we
+	 * will be able to successfully search for this modification
+	 * now
+	 */
+
+	search_dn = ldb_dn_new_fmt(search_test_ctx,
+				   search_test_ctx->ldb_test_ctx->ldb,
+				   "cn=test_search_cn,"
+				   "dc=search_test_entry");
+
+	ret = ldb_search(search_test_ctx->ldb_test_ctx->ldb,
+			 search_test_ctx,
+			 &res2, search_dn, LDB_SCOPE_BASE, NULL,
+			 "filterAttr=TRUE");
+	assert_int_equal(ret, 0);
+
+	/* We got the result */
+	assert_int_equal(res2->count, 1);
+
 
 }
 
-- 
2.11.0


From bb297536e8eb008e9e51a8c01a6eb6637784d614 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 11 Apr 2017 17:50:08 +0200
Subject: [PATCH 11/18] TODO: ldb: version 1.1.31

* fix ldb_tdb locking (performance) problems
* fix ldb_tdb search inconsistencies
* add cmocka based tests for the locking issues

TODO: review...
---
 lib/ldb/ABI/ldb-1.1.31.sigs            | 274 +++++++++++++++++++++++++++++++++
 lib/ldb/ABI/pyldb-util-1.1.31.sigs     |   2 +
 lib/ldb/ABI/pyldb-util.py3-1.1.31.sigs |   2 +
 lib/ldb/wscript                        |   2 +-
 4 files changed, 279 insertions(+), 1 deletion(-)
 create mode 100644 lib/ldb/ABI/ldb-1.1.31.sigs
 create mode 100644 lib/ldb/ABI/pyldb-util-1.1.31.sigs
 create mode 100644 lib/ldb/ABI/pyldb-util.py3-1.1.31.sigs

diff --git a/lib/ldb/ABI/ldb-1.1.31.sigs b/lib/ldb/ABI/ldb-1.1.31.sigs
new file mode 100644
index 00000000000..c6935a3938d
--- /dev/null
+++ b/lib/ldb/ABI/ldb-1.1.31.sigs
@@ -0,0 +1,274 @@
+ldb_add: int (struct ldb_context *, const struct ldb_message *)
+ldb_any_comparison: int (struct ldb_context *, void *, ldb_attr_handler_t, const struct ldb_val *, const struct ldb_val *)
+ldb_asprintf_errstring: void (struct ldb_context *, const char *, ...)
+ldb_attr_casefold: char *(TALLOC_CTX *, const char *)
+ldb_attr_dn: int (const char *)
+ldb_attr_in_list: int (const char * const *, const char *)
+ldb_attr_list_copy: const char **(TALLOC_CTX *, const char * const *)
+ldb_attr_list_copy_add: const char **(TALLOC_CTX *, const char * const *, const char *)
+ldb_base64_decode: int (char *)
+ldb_base64_encode: char *(TALLOC_CTX *, const char *, int)
+ldb_binary_decode: struct ldb_val (TALLOC_CTX *, const char *)
+ldb_binary_encode: char *(TALLOC_CTX *, struct ldb_val)
+ldb_binary_encode_string: char *(TALLOC_CTX *, const char *)
+ldb_build_add_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_del_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_extended_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const char *, void *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_mod_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_rename_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, struct ldb_dn *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, const char *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_build_search_req_ex: int (struct ldb_request **, struct ldb_context *, TALLOC_CTX *, struct ldb_dn *, enum ldb_scope, struct ldb_parse_tree *, const char * const *, struct ldb_control **, void *, ldb_request_callback_t, struct ldb_request *)
+ldb_casefold: char *(struct ldb_context *, TALLOC_CTX *, const char *, size_t)
+ldb_casefold_default: char *(void *, TALLOC_CTX *, const char *, size_t)
+ldb_check_critical_controls: int (struct ldb_control **)
+ldb_comparison_binary: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_comparison_fold: int (struct ldb_context *, void *, const struct ldb_val *, const struct ldb_val *)
+ldb_connect: int (struct ldb_context *, const char *, unsigned int, const char **)
+ldb_control_to_string: char *(TALLOC_CTX *, const struct ldb_control *)
+ldb_controls_except_specified: struct ldb_control **(struct ldb_control **, TALLOC_CTX *, struct ldb_control *)
+ldb_debug: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_debug_add: void (struct ldb_context *, const char *, ...)
+ldb_debug_end: void (struct ldb_context *, enum ldb_debug_level)
+ldb_debug_set: void (struct ldb_context *, enum ldb_debug_level, const char *, ...)
+ldb_delete: int (struct ldb_context *, struct ldb_dn *)
+ldb_dn_add_base: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_base_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_add_child: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_add_child_fmt: bool (struct ldb_dn *, const char *, ...)
+ldb_dn_alloc_casefold: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_alloc_linearized: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_ex_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_canonical_string: char *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_check_local: bool (struct ldb_module *, struct ldb_dn *)
+ldb_dn_check_special: bool (struct ldb_dn *, const char *)
+ldb_dn_compare: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_compare_base: int (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_copy: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_escape_value: char *(TALLOC_CTX *, struct ldb_val)
+ldb_dn_extended_add_syntax: int (struct ldb_context *, unsigned int, const struct ldb_dn_extended_syntax *)
+ldb_dn_extended_filter: void (struct ldb_dn *, const char * const *)
+ldb_dn_extended_syntax_by_name: const struct ldb_dn_extended_syntax *(struct ldb_context *, const char *)
+ldb_dn_from_ldb_val: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const struct ldb_val *)
+ldb_dn_get_casefold: const char *(struct ldb_dn *)
+ldb_dn_get_comp_num: int (struct ldb_dn *)
+ldb_dn_get_component_name: const char *(struct ldb_dn *, unsigned int)
+ldb_dn_get_component_val: const struct ldb_val *(struct ldb_dn *, unsigned int)
+ldb_dn_get_extended_comp_num: int (struct ldb_dn *)
+ldb_dn_get_extended_component: const struct ldb_val *(struct ldb_dn *, const char *)
+ldb_dn_get_extended_linearized: char *(TALLOC_CTX *, struct ldb_dn *, int)
+ldb_dn_get_ldb_context: struct ldb_context *(struct ldb_dn *)
+ldb_dn_get_linearized: const char *(struct ldb_dn *)
+ldb_dn_get_parent: struct ldb_dn *(TALLOC_CTX *, struct ldb_dn *)
+ldb_dn_get_rdn_name: const char *(struct ldb_dn *)
+ldb_dn_get_rdn_val: const struct ldb_val *(struct ldb_dn *)
+ldb_dn_has_extended: bool (struct ldb_dn *)
+ldb_dn_is_null: bool (struct ldb_dn *)
+ldb_dn_is_special: bool (struct ldb_dn *)
+ldb_dn_is_valid: bool (struct ldb_dn *)
+ldb_dn_map_local: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_rebase_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_map_remote: struct ldb_dn *(struct ldb_module *, void *, struct ldb_dn *)
+ldb_dn_minimise: bool (struct ldb_dn *)
+ldb_dn_new: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *)
+ldb_dn_new_fmt: struct ldb_dn *(TALLOC_CTX *, struct ldb_context *, const char *, ...)
+ldb_dn_remove_base_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_child_components: bool (struct ldb_dn *, unsigned int)
+ldb_dn_remove_extended_components: void (struct ldb_dn *)
+ldb_dn_replace_components: bool (struct ldb_dn *, struct ldb_dn *)
+ldb_dn_set_component: int (struct ldb_dn *, int, const char *, const struct ldb_val)
+ldb_dn_set_extended_component: int (struct ldb_dn *, const char *, const struct ldb_val *)
+ldb_dn_update_components: int (struct ldb_dn *, const struct ldb_dn *)
+ldb_dn_validate: bool (struct ldb_dn *)
+ldb_dump_results: void (struct ldb_context *, struct ldb_result *, FILE *)
+ldb_error_at: int (struct ldb_context *, int, const char *, const char *, int)
+ldb_errstring: const char *(struct ldb_context *)
+ldb_extended: int (struct ldb_context *, const char *, void *, struct ldb_result **)
+ldb_extended_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_filter_from_tree: char *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_get_config_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_create_perms: unsigned int (struct ldb_context *)
+ldb_get_default_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_event_context: struct tevent_context *(struct ldb_context *)
+ldb_get_flags: unsigned int (struct ldb_context *)
+ldb_get_opaque: void *(struct ldb_context *, const char *)
+ldb_get_root_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_get_schema_basedn: struct ldb_dn *(struct ldb_context *)
+ldb_global_init: int (void)
+ldb_handle_get_event_context: struct tevent_context *(struct ldb_handle *)
+ldb_handle_new: struct ldb_handle *(TALLOC_CTX *, struct ldb_context *)
+ldb_handle_use_global_event_context: void (struct ldb_handle *)
+ldb_handler_copy: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_handler_fold: int (struct ldb_context *, void *, const struct ldb_val *, struct ldb_val *)
+ldb_init: struct ldb_context *(TALLOC_CTX *, struct tevent_context *)
+ldb_ldif_message_string: char *(struct ldb_context *, TALLOC_CTX *, enum ldb_changetype, const struct ldb_message *)
+ldb_ldif_parse_modrdn: int (struct ldb_context *, const struct ldb_ldif *, TALLOC_CTX *, struct ldb_dn **, struct ldb_dn **, bool *, struct ldb_dn **, struct ldb_dn **)
+ldb_ldif_read: struct ldb_ldif *(struct ldb_context *, int (*)(void *), void *)
+ldb_ldif_read_file: struct ldb_ldif *(struct ldb_context *, FILE *)
+ldb_ldif_read_file_state: struct ldb_ldif *(struct ldb_context *, struct ldif_read_file_state *)
+ldb_ldif_read_free: void (struct ldb_context *, struct ldb_ldif *)
+ldb_ldif_read_string: struct ldb_ldif *(struct ldb_context *, const char **)
+ldb_ldif_write: int (struct ldb_context *, int (*)(void *, const char *, ...), void *, const struct ldb_ldif *)
+ldb_ldif_write_file: int (struct ldb_context *, FILE *, const struct ldb_ldif *)
+ldb_ldif_write_redacted_trace_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_ldif_write_string: char *(struct ldb_context *, TALLOC_CTX *, const struct ldb_ldif *)
+ldb_load_modules: int (struct ldb_context *, const char **)
+ldb_map_add: int (struct ldb_module *, struct ldb_request *)
+ldb_map_delete: int (struct ldb_module *, struct ldb_request *)
+ldb_map_init: int (struct ldb_module *, const struct ldb_map_attribute *, const struct ldb_map_objectclass *, const char * const *, const char *, const char *)
+ldb_map_modify: int (struct ldb_module *, struct ldb_request *)
+ldb_map_rename: int (struct ldb_module *, struct ldb_request *)
+ldb_map_search: int (struct ldb_module *, struct ldb_request *)
+ldb_match_msg: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope)
+ldb_match_msg_error: int (struct ldb_context *, const struct ldb_message *, const struct ldb_parse_tree *, struct ldb_dn *, enum ldb_scope, bool *)
+ldb_match_msg_objectclass: int (const struct ldb_message *, const char *)
+ldb_mod_register_control: int (struct ldb_module *, const char *)
+ldb_modify: int (struct ldb_context *, const struct ldb_message *)
+ldb_modify_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_module_call_chain: char *(struct ldb_request *, TALLOC_CTX *)
+ldb_module_connect_backend: int (struct ldb_context *, const char *, const char **, struct ldb_module **)
+ldb_module_done: int (struct ldb_request *, struct ldb_control **, struct ldb_extended *, int)
+ldb_module_flags: uint32_t (struct ldb_context *)
+ldb_module_get_ctx: struct ldb_context *(struct ldb_module *)
+ldb_module_get_name: const char *(struct ldb_module *)
+ldb_module_get_ops: const struct ldb_module_ops *(struct ldb_module *)
+ldb_module_get_private: void *(struct ldb_module *)
+ldb_module_init_chain: int (struct ldb_context *, struct ldb_module *)
+ldb_module_load_list: int (struct ldb_context *, const char **, struct ldb_module *, struct ldb_module **)
+ldb_module_new: struct ldb_module *(TALLOC_CTX *, struct ldb_context *, const char *, const struct ldb_module_ops *)
+ldb_module_next: struct ldb_module *(struct ldb_module *)
+ldb_module_popt_options: struct poptOption **(struct ldb_context *)
+ldb_module_send_entry: int (struct ldb_request *, struct ldb_message *, struct ldb_control **)
+ldb_module_send_referral: int (struct ldb_request *, char *)
+ldb_module_set_next: void (struct ldb_module *, struct ldb_module *)
+ldb_module_set_private: void (struct ldb_module *, void *)
+ldb_modules_hook: int (struct ldb_context *, enum ldb_module_hook_type)
+ldb_modules_list_from_string: const char **(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_modules_load: int (const char *, const char *)
+ldb_msg_add: int (struct ldb_message *, const struct ldb_message_element *, int)
+ldb_msg_add_empty: int (struct ldb_message *, const char *, int, struct ldb_message_element **)
+ldb_msg_add_fmt: int (struct ldb_message *, const char *, const char *, ...)
+ldb_msg_add_linearized_dn: int (struct ldb_message *, const char *, struct ldb_dn *)
+ldb_msg_add_steal_string: int (struct ldb_message *, const char *, char *)
+ldb_msg_add_steal_value: int (struct ldb_message *, const char *, struct ldb_val *)
+ldb_msg_add_string: int (struct ldb_message *, const char *, const char *)
+ldb_msg_add_value: int (struct ldb_message *, const char *, const struct ldb_val *, struct ldb_message_element **)
+ldb_msg_canonicalize: struct ldb_message *(struct ldb_context *, const struct ldb_message *)
+ldb_msg_check_string_attribute: int (const struct ldb_message *, const char *, const char *)
+ldb_msg_copy: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_copy_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_copy_shallow: struct ldb_message *(TALLOC_CTX *, const struct ldb_message *)
+ldb_msg_diff: struct ldb_message *(struct ldb_context *, struct ldb_message *, struct ldb_message *)
+ldb_msg_difference: int (struct ldb_context *, TALLOC_CTX *, struct ldb_message *, struct ldb_message *, struct ldb_message **)
+ldb_msg_element_compare: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_compare_name: int (struct ldb_message_element *, struct ldb_message_element *)
+ldb_msg_element_equal_ordered: bool (const struct ldb_message_element *, const struct ldb_message_element *)
+ldb_msg_find_attr_as_bool: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_dn: struct ldb_dn *(struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, const char *)
+ldb_msg_find_attr_as_double: double (const struct ldb_message *, const char *, double)
+ldb_msg_find_attr_as_int: int (const struct ldb_message *, const char *, int)
+ldb_msg_find_attr_as_int64: int64_t (const struct ldb_message *, const char *, int64_t)
+ldb_msg_find_attr_as_string: const char *(const struct ldb_message *, const char *, const char *)
+ldb_msg_find_attr_as_uint: unsigned int (const struct ldb_message *, const char *, unsigned int)
+ldb_msg_find_attr_as_uint64: uint64_t (const struct ldb_message *, const char *, uint64_t)
+ldb_msg_find_element: struct ldb_message_element *(const struct ldb_message *, const char *)
+ldb_msg_find_ldb_val: const struct ldb_val *(const struct ldb_message *, const char *)
+ldb_msg_find_val: struct ldb_val *(const struct ldb_message_element *, struct ldb_val *)
+ldb_msg_new: struct ldb_message *(TALLOC_CTX *)
+ldb_msg_normalize: int (struct ldb_context *, TALLOC_CTX *, const struct ldb_message *, struct ldb_message **)
+ldb_msg_remove_attr: void (struct ldb_message *, const char *)
+ldb_msg_remove_element: void (struct ldb_message *, struct ldb_message_element *)
+ldb_msg_rename_attr: int (struct ldb_message *, const char *, const char *)
+ldb_msg_sanity_check: int (struct ldb_context *, const struct ldb_message *)
+ldb_msg_sort_elements: void (struct ldb_message *)
+ldb_next_del_trans: int (struct ldb_module *)
+ldb_next_end_trans: int (struct ldb_module *)
+ldb_next_init: int (struct ldb_module *)
+ldb_next_prepare_commit: int (struct ldb_module *)
+ldb_next_read_lock: int (struct ldb_module *)
+ldb_next_read_unlock: int (struct ldb_module *)
+ldb_next_remote_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_request: int (struct ldb_module *, struct ldb_request *)
+ldb_next_start_trans: int (struct ldb_module *)
+ldb_op_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_options_find: const char *(struct ldb_context *, const char **, const char *)
+ldb_pack_data: int (struct ldb_context *, const struct ldb_message *, struct ldb_val *)
+ldb_parse_control_from_string: struct ldb_control *(struct ldb_context *, TALLOC_CTX *, const char *)
+ldb_parse_control_strings: struct ldb_control **(struct ldb_context *, TALLOC_CTX *, const char **)
+ldb_parse_tree: struct ldb_parse_tree *(TALLOC_CTX *, const char *)
+ldb_parse_tree_attr_replace: void (struct ldb_parse_tree *, const char *, const char *)
+ldb_parse_tree_copy_shallow: struct ldb_parse_tree *(TALLOC_CTX *, const struct ldb_parse_tree *)
+ldb_parse_tree_walk: int (struct ldb_parse_tree *, int (*)(struct ldb_parse_tree *, void *), void *)
+ldb_qsort: void (void * const, size_t, size_t, void *, ldb_qsort_cmp_fn_t)
+ldb_register_backend: int (const char *, ldb_connect_fn, bool)
+ldb_register_extended_match_rule: int (struct ldb_context *, const struct ldb_extended_match_rule *)
+ldb_register_hook: int (ldb_hook_fn)
+ldb_register_module: int (const struct ldb_module_ops *)
+ldb_rename: int (struct ldb_context *, struct ldb_dn *, struct ldb_dn *)
+ldb_reply_add_control: int (struct ldb_reply *, const char *, bool, void *)
+ldb_reply_get_control: struct ldb_control *(struct ldb_reply *, const char *)
+ldb_req_get_custom_flags: uint32_t (struct ldb_request *)
+ldb_req_is_untrusted: bool (struct ldb_request *)
+ldb_req_location: const char *(struct ldb_request *)
+ldb_req_mark_trusted: void (struct ldb_request *)
+ldb_req_mark_untrusted: void (struct ldb_request *)
+ldb_req_set_custom_flags: void (struct ldb_request *, uint32_t)
+ldb_req_set_location: void (struct ldb_request *, const char *)
+ldb_request: int (struct ldb_context *, struct ldb_request *)
+ldb_request_add_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_done: int (struct ldb_request *, int)
+ldb_request_get_control: struct ldb_control *(struct ldb_request *, const char *)
+ldb_request_get_status: int (struct ldb_request *)
+ldb_request_replace_control: int (struct ldb_request *, const char *, bool, void *)
+ldb_request_set_state: void (struct ldb_request *, int)
+ldb_reset_err_string: void (struct ldb_context *)
+ldb_save_controls: int (struct ldb_control *, struct ldb_request *, struct ldb_control ***)
+ldb_schema_attribute_add: int (struct ldb_context *, const char *, unsigned int, const char *)
+ldb_schema_attribute_add_with_syntax: int (struct ldb_context *, const char *, unsigned int, const struct ldb_schema_syntax *)
+ldb_schema_attribute_by_name: const struct ldb_schema_attribute *(struct ldb_context *, const char *)
+ldb_schema_attribute_fill_with_syntax: int (struct ldb_context *, TALLOC_CTX *, const char *, unsigned int, const struct ldb_schema_syntax *, struct ldb_schema_attribute *)
+ldb_schema_attribute_remove: void (struct ldb_context *, const char *)
+ldb_schema_attribute_remove_flagged: void (struct ldb_context *, unsigned int)
+ldb_schema_attribute_set_override_handler: void (struct ldb_context *, ldb_attribute_handler_override_fn_t, void *)
+ldb_schema_set_override_indexlist: void (struct ldb_context *, bool)
+ldb_search: int (struct ldb_context *, TALLOC_CTX *, struct ldb_result **, struct ldb_dn *, enum ldb_scope, const char * const *, const char *, ...)
+ldb_search_default_callback: int (struct ldb_request *, struct ldb_reply *)
+ldb_sequence_number: int (struct ldb_context *, enum ldb_sequence_type, uint64_t *)
+ldb_set_create_perms: void (struct ldb_context *, unsigned int)
+ldb_set_debug: int (struct ldb_context *, void (*)(void *, enum ldb_debug_level, const char *, va_list), void *)
+ldb_set_debug_stderr: int (struct ldb_context *)
+ldb_set_default_dns: void (struct ldb_context *)
+ldb_set_errstring: void (struct ldb_context *, const char *)
+ldb_set_event_context: void (struct ldb_context *, struct tevent_context *)
+ldb_set_flags: void (struct ldb_context *, unsigned int)
+ldb_set_modules_dir: void (struct ldb_context *, const char *)
+ldb_set_opaque: int (struct ldb_context *, const char *, void *)
+ldb_set_require_private_event_context: void (struct ldb_context *)
+ldb_set_timeout: int (struct ldb_context *, struct ldb_request *, int)
+ldb_set_timeout_from_prev_req: int (struct ldb_context *, struct ldb_request *, struct ldb_request *)
+ldb_set_utf8_default: void (struct ldb_context *)
+ldb_set_utf8_fns: void (struct ldb_context *, void *, char *(*)(void *, void *, const char *, size_t))
+ldb_setup_wellknown_attributes: int (struct ldb_context *)
+ldb_should_b64_encode: int (struct ldb_context *, const struct ldb_val *)
+ldb_standard_syntax_by_name: const struct ldb_schema_syntax *(struct ldb_context *, const char *)
+ldb_strerror: const char *(int)
+ldb_string_to_time: time_t (const char *)
+ldb_string_utc_to_time: time_t (const char *)
+ldb_timestring: char *(TALLOC_CTX *, time_t)
+ldb_timestring_utc: char *(TALLOC_CTX *, time_t)
+ldb_transaction_cancel: int (struct ldb_context *)
+ldb_transaction_cancel_noerr: int (struct ldb_context *)
+ldb_transaction_commit: int (struct ldb_context *)
+ldb_transaction_prepare_commit: int (struct ldb_context *)
+ldb_transaction_start: int (struct ldb_context *)
+ldb_unpack_data: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *)
+ldb_unpack_data_only_attr_list: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int *)
+ldb_unpack_data_only_attr_list_flags: int (struct ldb_context *, const struct ldb_val *, struct ldb_message *, const char * const *, unsigned int, unsigned int, unsigned int *)
+ldb_val_dup: struct ldb_val (TALLOC_CTX *, const struct ldb_val *)
+ldb_val_equal_exact: int (const struct ldb_val *, const struct ldb_val *)
+ldb_val_map_local: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_map_remote: struct ldb_val (struct ldb_module *, void *, const struct ldb_map_attribute *, const struct ldb_val *)
+ldb_val_string_cmp: int (const struct ldb_val *, const char *)
+ldb_val_to_time: int (const struct ldb_val *, time_t *)
+ldb_valid_attr_name: int (const char *)
+ldb_vdebug: void (struct ldb_context *, enum ldb_debug_level, const char *, va_list)
+ldb_wait: int (struct ldb_handle *, enum ldb_wait_type)
diff --git a/lib/ldb/ABI/pyldb-util-1.1.31.sigs b/lib/ldb/ABI/pyldb-util-1.1.31.sigs
new file mode 100644
index 00000000000..74d6719d2bc
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util-1.1.31.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/ABI/pyldb-util.py3-1.1.31.sigs b/lib/ldb/ABI/pyldb-util.py3-1.1.31.sigs
new file mode 100644
index 00000000000..74d6719d2bc
--- /dev/null
+++ b/lib/ldb/ABI/pyldb-util.py3-1.1.31.sigs
@@ -0,0 +1,2 @@
+pyldb_Dn_FromDn: PyObject *(struct ldb_dn *)
+pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 635a787b5d6..da8a93a9b20 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'ldb'
-VERSION = '1.1.30'
+VERSION = '1.1.31'
 
 blddir = 'bin'
 
-- 
2.11.0


From 72d23880d3c6512d9694202b549cbaf6ac642192 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 23 May 2017 15:11:59 +1200
Subject: [PATCH 12/18] dsdb: Teach the Samba partition module how to lock all
 the DB backends

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/partition.c | 149 ++++++++++++++++++++++++++++-
 1 file changed, 148 insertions(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 08a959cabb5..5620a34bfc8 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -814,7 +814,7 @@ static int partition_start_trans(struct ldb_module *module)
 		ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_start_trans() -> (metadata partition)");
 	}
 
-	/* This order must match that in prepare_commit() */
+	/* This order must match that in prepare_commit() and lock_backend */
 	ret = ldb_next_start_trans(module);
 	if (ret != LDB_SUCCESS) {
 		return ret;
@@ -1144,6 +1144,151 @@ static int partition_sequence_number(struct ldb_module *module, struct ldb_reque
 	return ldb_module_done(req, NULL, ext, LDB_SUCCESS);
 }
 
+/* lock or unlock all the backends */
+static int partition_lock(struct ldb_module *module)
+{
+	int i;
+	int ret;
+	int ret2;
+	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct partition_private_data *data = talloc_get_type(ldb_module_get_private(module),
+							      struct partition_private_data);
+	if (ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING) {
+		ldb_debug(ldb, LDB_DEBUG_TRACE, "partition_lock_backend() -> (metadata partition)");
+	}
+
+	/*
+	 * It is important to only do this for LOCK because:
+	 * - we don't want to unlock what we did not lock
+	 *
+	 * - we don't want to make a new lock on the sam.ldb
+	 *   (triggered inside this routine due to the seq num check)
+	 *   during an unlock phase as that will violate the lock
+	 *   ordering
+	 */
+
+	/*
+	 * This will lock the metadata partition (sam.ldb) and
+	 * will also call event loops, so we do it before we
+	 * get the whole db lock.
+		 */
+	ret = partition_reload_if_required(module, data, NULL);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	/*
+	 * This order must match that in prepare_commit(), start with
+	 * the metadata partition (sam.ldb) lock
+	 */
+	ret = ldb_next_read_lock(module);
+	if (ret != LDB_SUCCESS) {
+		ldb_debug_set(ldb,
+			      LDB_DEBUG_FATAL,
+			      "Failed to lock db: %s / %s for metadata partition",
+			      ldb_errstring(ldb),
+			      ldb_strerror(ret));
+
+		return ret;
+	}
+
+	for (i=0; data && data->partitions && data->partitions[i]; i++) {
+		if ((module && ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING)) {
+			ldb_debug(ldb, LDB_DEBUG_TRACE,
+				  "partition_lock_backend() -> %s",
+				  ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+		}
+		ret = ldb_next_read_lock(data->partitions[i]->module);
+		if (ret == LDB_SUCCESS) {
+			continue;
+		}
+
+		ldb_debug_set(ldb,
+			      LDB_DEBUG_FATAL,
+			      "Failed to lock db: %s / %s for %s",
+			      ldb_errstring(ldb),
+			      ldb_strerror(ret),
+			      ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+
+		/* Back it out, if it fails on one */
+		for (i--; i >= 0; i--) {
+			ret2 = ldb_next_read_unlock(data->partitions[i]->module);
+			if (ret2 != LDB_SUCCESS) {
+				ldb_debug(ldb,
+					  LDB_DEBUG_FATAL,
+					  "Failed to unlock db: %s / %s",
+					  ldb_errstring(ldb),
+					  ldb_strerror(ret2));
+			}
+		}
+		ret2 = ldb_next_read_unlock(module);
+		if (ret2 != LDB_SUCCESS) {
+			ldb_debug(ldb,
+				  LDB_DEBUG_FATAL,
+				  "Failed to unlock db: %s / %s",
+				  ldb_errstring(ldb),
+				  ldb_strerror(ret2));
+		}
+		return ret;
+	}
+
+	return LDB_SUCCESS;
+}
+
+static int partition_unlock(struct ldb_module *module)
+{
+	int i;
+	int ret;
+	int ret2;
+	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct partition_private_data *data = talloc_get_type(ldb_module_get_private(module),
+							      struct partition_private_data);
+	if (ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING) {
+		ldb_debug(ldb, LDB_DEBUG_TRACE, "partition_unlock_backend() -> (metadata partition)");
+	}
+
+	/*
+	 * This order must match that in read_lock(), start with
+	 * the metadata partition (sam.ldb) lock
+	 */
+	ret = ldb_next_read_unlock(module);
+	if (ret != LDB_SUCCESS) {
+		ldb_debug_set(ldb,
+			      LDB_DEBUG_FATAL,
+			      "Failed to unlock db: %s / %s for metadata partition",
+			      ldb_errstring(ldb),
+			      ldb_strerror(ret));
+	}
+
+	for (i=0; data && data->partitions && data->partitions[i]; i++) {
+		if ((module && ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING)) {
+			ldb_debug(ldb, LDB_DEBUG_TRACE,
+				  "partition_unlock_backend() -> %s",
+				  ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+		}
+		ret2 = ldb_next_read_unlock(data->partitions[i]->module);
+		if (ret2 != LDB_SUCCESS) {
+			ldb_debug_set(ldb,
+				      LDB_DEBUG_FATAL,
+				      "Failed to lock db: %s / %s for %s",
+				      ldb_errstring(ldb),
+				      ldb_strerror(ret),
+				      ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+
+			/*
+			 * Don't overwrite the original failure code
+			 * if there was one
+			 */
+			if (ret == LDB_SUCCESS) {
+				ret = ret2;
+			}
+		}
+	}
+
+	return ret;
+}
+
+
 /* extended */
 static int partition_extended(struct ldb_module *module, struct ldb_request *req)
 {
@@ -1198,6 +1343,8 @@ static const struct ldb_module_ops ldb_partition_module_ops = {
 	.prepare_commit    = partition_prepare_commit,
 	.end_transaction   = partition_end_trans,
 	.del_transaction   = partition_del_trans,
+	.read_lock         = partition_lock,
+	.read_unlock       = partition_unlock
 };
 
 int ldb_partition_module_init(const char *version)
-- 
2.11.0


From ede84a87782c48dd41035e9ef294fbcfc8f51d59 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 7 Jun 2017 10:44:50 +1200
Subject: [PATCH 13/18] dsdb: Do not run dsdb_replace() on the calculated
 difference between old and new schema

We can set the database @INDEXLIST and @ATTRIBUTES to the full calculated
values, not the difference, and let the ldb layer work it out under the
transaction lock.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/schema/schema_set.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index 977c9e339b6..df27e19a944 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -174,7 +174,12 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
 			goto op_error;
 		}
 		if (mod_msg->num_elements > 0) {
-			ret = dsdb_replace(ldb, mod_msg, 0);
+			/*
+			 * Do the replace with the constructed message,
+			 * to avoid needing a lock between this search
+			 * and the replace
+			 */
+			ret = dsdb_replace(ldb, msg, 0);
 		}
 		talloc_free(mod_msg);
 	}
@@ -210,7 +215,12 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
 			goto op_error;
 		}
 		if (mod_msg->num_elements > 0) {
-			ret = dsdb_replace(ldb, mod_msg, 0);
+			/*
+			 * Do the replace with the constructed message,
+			 * to avoid needing a lock between this search
+			 * and the replace
+			 */
+			ret = dsdb_replace(ldb, msg_idx, 0);
 		}
 		talloc_free(mod_msg);
 	}
-- 
2.11.0


From afc40bcb5b0b2c072fd85feb4a965d039139a55a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sat, 10 Jun 2017 19:23:34 +1200
Subject: [PATCH 14/18] dsdb: Add comment explaining requirements on
 DSDB_EXTENDED_SCHEMA_UPDATE_NOW_OID

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/samdb/ldb_modules/schema_load.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index 3fba97e4f20..92160943a89 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -530,12 +530,13 @@ static int schema_load_del_transaction(struct ldb_module *module)
 	return ldb_next_del_trans(module);
 }
 
+/* This is called in a transaction held by the callers */
 static int schema_load_extended(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	struct dsdb_schema *schema;
 	int ret;
-	
+
 	if (strcmp(req->op.extended.oid, DSDB_EXTENDED_SCHEMA_UPDATE_NOW_OID) != 0) {
 		return ldb_next_request(module, req);
 	}
-- 
2.11.0


From 22559a91ad6fd4e10c50d9d3d7ca8670def1a90d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 11 Jun 2017 23:19:01 +0200
Subject: [PATCH 15/18] krb5_wrap: handle KRB5_ERR_HOST_REALM_UNKNOWN in
 smb_krb5_get_realm_from_hostname()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 lib/krb5_wrap/krb5_samba.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c
index 2e43f797144..0c8b402c21e 100644
--- a/lib/krb5_wrap/krb5_samba.c
+++ b/lib/krb5_wrap/krb5_samba.c
@@ -2669,6 +2669,10 @@ char *smb_krb5_get_realm_from_hostname(TALLOC_CTX *mem_ctx,
 	}
 
 	kerr = krb5_get_host_realm(ctx, hostname, &realm_list);
+	if (kerr == KRB5_ERR_HOST_REALM_UNKNOWN) {
+		realm_list = NULL;
+		kerr = 0;
+	}
 	if (kerr != 0) {
 		DEBUG(3,("kerberos_get_realm_from_hostname %s: "
 			"failed %s\n",
-- 
2.11.0


From 73a1990cf9cb976c86a998e1201732660e11279c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 14 Jun 2017 13:11:56 +1200
Subject: [PATCH 16/18] selftest: Fix failure message in dsdb_schema_info

The rename changes the CN, not the lDAPDisplayName

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/tests/python/dsdb_schema_info.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/tests/python/dsdb_schema_info.py b/source4/dsdb/tests/python/dsdb_schema_info.py
index 0ae95b3836b..f3452d67acb 100755
--- a/source4/dsdb/tests/python/dsdb_schema_info.py
+++ b/source4/dsdb/tests/python/dsdb_schema_info.py
@@ -141,7 +141,7 @@ systemOnly: FALSE
         try:
             self.sam_db.rename(attr_dn, attr_dn_new)
         except LdbError, (num, _):
-            self.fail("failed to change lDAPDisplayName for %s: %s" % (attr_name, _))
+            self.fail("failed to change CN for %s: %s" % (attr_name, _))
 
         # compare resulting schemaInfo
         schi_after = self._getSchemaInfo()
@@ -187,7 +187,7 @@ systemOnly: FALSE
         try:
             self.sam_db.rename(class_dn, class_dn_new)
         except LdbError, (num, _):
-            self.fail("failed to change lDAPDisplayName for %s: %s" % (class_name, _))
+            self.fail("failed to change CN for %s: %s" % (class_name, _))
 
         # compare resulting schemaInfo
         schi_after = self._getSchemaInfo()
-- 
2.11.0


From 5676916e3bbdd2e00d0a1604e427271d0a3782aa Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 16:19:17 +1200
Subject: [PATCH 17/18] selftest: Correctly print message when nbt is not up in
 20 seconds

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12843
---
 selftest/target/Samba4.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 316ef8346d9..c52c99b1759 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -207,7 +207,7 @@ sub wait_for_start($$)
 		}
 		$count++;
 	} while ($ret != 0 && $count < 20);
-	if ($count == 10) {
+	if ($count == 20) {
 		warn("nbt not reachable after 20 retries\n");
 		teardown_env($self, $testenv_vars);
 		return 0;
-- 
2.11.0


From 305b2220242e1d1a6032fca498b40e24b96bb0aa Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Jun 2017 16:20:11 +1200
Subject: [PATCH 18/18] selftest: Also wait for winbindd to start

This ensures that the posixacl.py test does not race against winbindd starting up and so
give wrong mappings

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12843
---
 selftest/target/Samba4.pm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index c52c99b1759..ea81d7dd9da 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -245,6 +245,28 @@ sub wait_for_start($$)
 			sleep(1);
 		}
 	}
+
+	my $wbinfo =  Samba::bindir_path($self, "wbinfo");
+
+	$count = 0;
+	do {
+		my $cmd = "NSS_WRAPPER_PASSWD=$testenv_vars->{NSS_WRAPPER_PASSWD} ";
+		$cmd .= "NSS_WRAPPER_GROUP=$testenv_vars->{NSS_WRAPPER_GROUP} ";
+		$cmd .= "SELFTEST_WINBINDD_SOCKET_DIR=$testenv_vars->{SELFTEST_WINBINDD_SOCKET_DIR} ";
+		$cmd .= "$wbinfo -p";
+		$ret = system($cmd);
+
+		if ($ret != 0) {
+			sleep(1);
+		}
+		$count++;
+	} while ($ret != 0 && $count < 20);
+	if ($count == 20) {
+		warn("winbind not reachable after 20 retries\n");
+		teardown_env($self, $testenv_vars);
+		return 0;
+	}
+
 	print $self->getlog_env($testenv_vars);
 
 	return $ret
-- 
2.11.0



More information about the samba-technical mailing list