[WIP][PATCH] Safe LDB locking for better replication
Andrew Bartlett
abartlet at samba.org
Wed Jun 14 05:19:14 UTC 2017
On Fri, 2017-06-09 at 09:55 +0200, Stefan Metzmacher via samba-
technical wrote:
> Am 09.06.2017 um 07:02 schrieb Andrew Bartlett:
> > I think this patch is almost finished (but I've thought that for a
> > long
> > time).
> >
> > Here is my patch to do whole-DB locking and TDB locking during the
> > search. I have it under test right now, which will almost
> > certainly
> > blow up, but I want to post it here for comment in the meantime.
> >
> > If correct, it should fix a number of 'missing objectclass'
> > replication
> > errors and flapping tests in autobuild.
> >
> > I still need to make the tests die (rather than hang) when used
> > without
> > the fixed TDB. No doubt I've missed other things too, but here is
> > is.
>
> I guess the wrong dsdb_replace() for the index mod_msg is one of the
> most important bugs, which we can push and backport now?
This turned out to be a red herring, but I have a patch in autoubild
that shows that our current behaviour is correct.
> Regarding the db lock around the high level search, I'd have a better
> feeling if we would make sure that any modifications would give an
> error
> while a search is running.
This is reinforced by the tdb layer refusing to grant a transaction
lock while the all-record lock is held.
From transaction.c:
transaction design:
- don't allow any locks to be held when a transaction starts,
otherwise we can end up with deadlock (plus lack of lock nesting
in posix locks would mean the lock is lost)
> Similar to tdb_traverse_read() where the
> callbacks are not allowed to do any modifications.
>
> I'm not sure it helps to track this on the ldb_context, as we can
> have multiple ldb_contexts within one process which make use of the
> same
> low level tdb_context.
I think follow, and this is why we ban nested event loops.
> I'm also not sure if we have to get the read locks on all partitions,
> wouldn't it be enough to get the lock just on the main sam.ldb file?
> As every search and transaction would always go through this first,
> it should be enough to have that as a guard against modifications
> and it will reduce the processing and syscall overhead.
If we don't take an all-record lock on the DB, then the traverse will
do a walking lock over the DB, which is very slow (this work started as
an effort to gain performance!).
> Finally given that the code in lib/ldb/common/ldb.c will be
> the only place that does the LOCK_DB and UNLOCK_DB calls,
> I'd say it's better to add new read_lock() and read_unlock() hooks to
> the module stack instead of using an extended operation, that would
> also
> simplify the code a lot.
It would, I only took this hideous approach on what I understood to be
your insistence, and will be glad to remove it and make it more like
the transaction design. :-)
> We could then have the coordination checks which make sure that the
> following constraints are enforced:
> 1. Only one top level search can run at a time! So the
> top level callbacks are not allowed to do any other searches.
> This guarantees that we just read_lock()/unlock() for one single
> search and don't get into a possible recursion/nesting hell again.
I'll have to think about this.
> 2. Once a top level read_lock() is done transaction_start() would
> fail
I think we get this already.
> 3. If a transaction is already started we skip the read_lock() call
> to
> the tdb level, but still make sure only one top level search can
> run
> at a time.
We sort of have this already, taking out the extra locks is nested at
the tdb layer.
> One thing to watch out for are the modify triggers in the rootdse
> module, which require the usage of the global event context and
> operate async using IRPC. We need to make sure that we don't
> have a tdb_transaction open while these are running.
We already cancel the transaction for that reason.
Thank you so very much for your thoughts, which I missed earlier.
Attached is my work in progress, with a set of patches that is starting
to pass tests as often as master.
We seem to be finding every other flapping test along the way (I got
lost into posixacl.py today, where it looks like things are stuck as
ID_TYPE_GID in the cache). The good news is we found and fixed another
replication bug today!
Anyway, with that proviso, here is the current set of patches - the set
that Garming has in autobuild and the remaining current WIP locking
set.
The current branch on catalyst git is ldb-safe-locking-single.
Further comments most welcome, and I plan to re-write it to use
read_lock() and read_unlock() operations tomorrow.
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 88550e614ab85b9f6a94cfa80257c5436a331898 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 9 Jun 2017 12:06:37 +1200
Subject: [PATCH 01/15] dsdb: Provide proper errors when
dsdb_schema_set_indices_and_attributes fails
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/schema/schema_set.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index fd48d279af2..977c9e339b6 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -184,6 +184,8 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
ret = LDB_SUCCESS;
}
if (ret != LDB_SUCCESS) {
+ DBG_ERR("Failed to set schema into @ATTRIBUTES: %s\n",
+ ldb_errstring(ldb));
talloc_free(mem_ctx);
return ret;
}
@@ -216,6 +218,12 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
/* We might be on a read-only DB */
ret = LDB_SUCCESS;
}
+
+ if (ret != LDB_SUCCESS) {
+ DBG_ERR("Failed to set schema into @INDEXLIST: %s\n",
+ ldb_errstring(ldb));
+ }
+
talloc_free(mem_ctx);
return ret;
--
2.11.0
From a1faff3b696a5f3af9fb3c87e39f9f3fb09da926 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 9 Jun 2017 14:07:40 +1200
Subject: [PATCH 02/15] ldb_tdb: Check for memory allocation failure in
ltdb_index_transaction_start()
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
lib/ldb/ldb_tdb/ldb_index.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 721ec1c9a6a..e1e54ba38db 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -54,6 +54,10 @@ int ltdb_index_transaction_start(struct ldb_module *module)
{
struct ltdb_private *ltdb = talloc_get_type(ldb_module_get_private(module), struct ltdb_private);
ltdb->idxptr = talloc_zero(ltdb, struct ltdb_idxptr);
+ if (ltdb->idxptr == NULL) {
+ return ldb_oom(ldb_module_get_ctx(module));
+ }
+
return LDB_SUCCESS;
}
--
2.11.0
From 49f5485ddd9b11931f474415716645c165fb10b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 9 Jun 2017 14:09:30 +1200
Subject: [PATCH 03/15] ldb_tdb: Remove the idxptr DB before we re-index
We do not want the cache or any of the values in it, we want to read the real DB
@INDEX: records.
This matters if a re-index is tiggered in the same transaction
as the modify of the values in the index. Otherwise we won't see
the old index record (it will not show up in the tdb_traverse)
and so fail to remove it.
That in turn can cause a spurious unqiue index violation.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
lib/ldb/ldb_tdb/ldb_index.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index e1e54ba38db..76f3cb392e2 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1655,6 +1655,18 @@ int ltdb_reindex(struct ldb_module *module)
return LDB_ERR_OPERATIONS_ERROR;
}
+ /*
+ * Ensure we read (and so remove) the entries from the real
+ * DB, no values stored so far are any use as we want to do a
+ * re-index
+ */
+ ltdb_index_transaction_cancel(module);
+
+ ret = ltdb_index_transaction_start(module);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
/* first traverse the database deleting any @INDEX records by
* putting NULL entries in the in-memory tdb
*/
--
2.11.0
From 722b7d962f813b6e599d0e6e83de20eb62742a8e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 9 Jun 2017 14:15:19 +1200
Subject: [PATCH 04/15] ldb_tdb: Improve logging on unique index violation
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
lib/ldb/ldb_tdb/ldb_index.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 76f3cb392e2..232bb4c16d9 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1179,9 +1179,22 @@ static int ltdb_index_add1(struct ldb_module *module, const char *dn,
if (list->count > 0 &&
a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX) {
- talloc_free(list);
+ /*
+ * We do not want to print info about a possibly
+ * confidential DN that the conflict was with in the
+ * user-visible error string
+ */
+ ldb_debug(ldb, LDB_DEBUG_WARNING,
+ __location__ ": unique index violation on %s in %s, "
+ "conficts with %*.*s in %s",
+ el->name, dn,
+ (int)list->dn[0].length,
+ (int)list->dn[0].length,
+ list->dn[0].data,
+ ldb_dn_get_linearized(dn_key));
ldb_asprintf_errstring(ldb, __location__ ": unique index violation on %s in %s",
el->name, dn);
+ talloc_free(list);
return LDB_ERR_ENTRY_ALREADY_EXISTS;
}
--
2.11.0
From 06dbb8a873d9b106bb4014b38df6866429bb9af9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 11 Apr 2017 17:21:20 +0200
Subject: [PATCH 05/15] tdb: add run-fcntl-deadlock test
This verifies the F_RDLCK => F_WRLCK upgrade logic in the kernel
for conflicting locks.
This is a standalone test to check the traverse_read vs.
allrecord_lock/prepare_commit interaction.
This is based on the example from
https://lists.samba.org/archive/samba-technical/2017-April/119861.html
from Douglas Bagnall <douglas.bagnall at catalyst.net.nz> and Volker Lendecke <vl at samba.org>.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
lib/tdb/test/run-fcntl-deadlock.c | 202 ++++++++++++++++++++++++++++++++++++++
lib/tdb/wscript | 1 +
2 files changed, 203 insertions(+)
create mode 100644 lib/tdb/test/run-fcntl-deadlock.c
diff --git a/lib/tdb/test/run-fcntl-deadlock.c b/lib/tdb/test/run-fcntl-deadlock.c
new file mode 100644
index 00000000000..0a328afaa50
--- /dev/null
+++ b/lib/tdb/test/run-fcntl-deadlock.c
@@ -0,0 +1,202 @@
+#include "../common/tdb_private.h"
+#include "../common/io.c"
+#include "../common/tdb.c"
+#include "../common/lock.c"
+#include "../common/freelist.c"
+#include "../common/traverse.c"
+#include "../common/transaction.c"
+#include "../common/error.c"
+#include "../common/open.c"
+#include "../common/check.c"
+#include "../common/hash.c"
+#include "../common/mutex.c"
+#include "replace.h"
+#include "system/filesys.h"
+#include "system/time.h"
+#include <errno.h>
+#include "tap-interface.h"
+
+/*
+ * This tests the low level locking requirement
+ * for the allrecord lock/prepare_commit and traverse_read interaction.
+ *
+ * The pattern with the traverse_read and prepare_commit interaction is
+ * the following:
+ *
+ * 1. transaction_start got the allrecord lock with F_RDLCK.
+ *
+ * 2. the traverse_read code walks the database in a sequence like this
+ * (per chain):
+ * 2.1 chainlock(chainX, F_RDLCK)
+ * 2.2 recordlock(chainX.record1, F_RDLCK)
+ * 2.3 chainunlock(chainX, F_RDLCK)
+ * 2.4 callback(chainX.record1)
+ * 2.5 chainlock(chainX, F_RDLCK)
+ * 2.6 recordunlock(chainX.record1, F_RDLCK)
+ * 2.7 recordlock(chainX.record2, F_RDLCK)
+ * 2.8 chainunlock(chainX, F_RDLCK)
+ * 2.9 callback(chainX.record2)
+ * 2.10 chainlock(chainX, F_RDLCK)
+ * 2.11 recordunlock(chainX.record2, F_RDLCK)
+ * 2.12 chainunlock(chainX, F_RDLCK)
+ * 2.13 goto next chain
+ *
+ * So it has always one record locked in F_RDLCK mode and tries to
+ * get the 2nd one before it releases the first one.
+ *
+ * 3. prepare_commit tries to upgrade the allrecord lock to F_RWLCK
+ * If that happens at the time of 2.4, the operation of
+ * 2.5 may deadlock with the allrecord lock upgrade.
+ * On Linux step 2.5 works in order to make some progress with the
+ * locking, but on solaris it might fail because the kernel
+ * wants to satisfy the 1st lock requester before the 2nd one.
+ *
+ * I think the first step is a standalone test that does this:
+ *
+ * process1: F_RDLCK for ofs=0 len=2
+ * process2: F_RDLCK for ofs=0 len=1
+ * process1: upgrade ofs=0 len=2 to F_RWLCK (in blocking mode)
+ * process2: F_RDLCK for ofs=1 len=1
+ * process2: unlock ofs=0 len=2
+ * process1: should continue at that point
+ *
+ * Such a test follows here...
+ */
+
+static int raw_fcntl_lock(int fd, int rw, off_t off, off_t len, bool waitflag)
+{
+ struct flock fl;
+ int cmd;
+ fl.l_type = rw;
+ fl.l_whence = SEEK_SET;
+ fl.l_start = off;
+ fl.l_len = len;
+ fl.l_pid = 0;
+
+ cmd = waitflag ? F_SETLKW : F_SETLK;
+
+ return fcntl(fd, cmd, &fl);
+}
+
+static int raw_fcntl_unlock(int fd, off_t off, off_t len)
+{
+ struct flock fl;
+ fl.l_type = F_UNLCK;
+ fl.l_whence = SEEK_SET;
+ fl.l_start = off;
+ fl.l_len = len;
+ fl.l_pid = 0;
+
+ return fcntl(fd, F_SETLKW, &fl);
+}
+
+
+int pipe_r;
+int pipe_w;
+char buf[2];
+
+static void expect_char(char c)
+{
+ read(pipe_r, buf, 1);
+ if (*buf != c) {
+ fail("We were expecting %c, but got %c", c, buf[0]);
+ }
+}
+
+static void send_char(char c)
+{
+ write(pipe_w, &c, 1);
+}
+
+
+int main(int argc, char *argv[])
+{
+ int process;
+ int fd;
+ const char *filename = "run-fcntl-deadlock.lck";
+ int pid;
+ int pipes_1_2[2];
+ int pipes_2_1[2];
+ int ret;
+
+ pipe(pipes_1_2);
+ pipe(pipes_2_1);
+ fd = open(filename, O_RDWR | O_CREAT, 0755);
+
+ pid = fork();
+ if (pid == 0) {
+ pipe_r = pipes_1_2[0];
+ pipe_w = pipes_2_1[1];
+ process = 2;
+ alarm(15);
+ } else {
+ pipe_r = pipes_2_1[0];
+ pipe_w = pipes_1_2[1];
+ process = 1;
+ alarm(15);
+ }
+
+ /* a: process1: F_RDLCK for ofs=0 len=2 */
+ if (process == 1) {
+ ret = raw_fcntl_lock(fd, F_RDLCK, 0, 2, true);
+ ok(ret == 0,
+ "process 1 lock ofs=0 len=2: %d - %s",
+ ret, strerror(errno));
+ diag("process 1 took read lock on range 0,2");
+ send_char('a');
+ }
+
+ /* process2: F_RDLCK for ofs=0 len=1 */
+ if (process == 2) {
+ expect_char('a');
+ ret = raw_fcntl_lock(fd, F_RDLCK, 0, 1, true);
+ ok(ret == 0,
+ "process 2 lock ofs=0 len=1: %d - %s",
+ ret, strerror(errno));;
+ diag("process 2 took read lock on range 0,1");
+ send_char('b');
+ }
+
+ /* process1: upgrade ofs=0 len=2 to F_RWLCK (in blocking mode) */
+ if (process == 1) {
+ expect_char('b');
+ send_char('c');
+ diag("process 1 starts upgrade on range 0,2");
+ ret = raw_fcntl_lock(fd, F_WRLCK, 0, 2, true);
+ ok(ret == 0,
+ "process 1 RW lock ofs=0 len=2: %d - %s",
+ ret, strerror(errno));
+ diag("process 1 got read upgrade done");
+ /* at this point process 1 is blocked on 2 releasing the
+ read lock */
+ }
+
+ /*
+ * process2: F_RDLCK for ofs=1 len=1
+ * process2: unlock ofs=0 len=2
+ */
+ if (process == 2) {
+ expect_char('c'); /* we know process 1 is *about* to lock */
+ sleep(1);
+ ret = raw_fcntl_lock(fd, F_RDLCK, 1, 1, true);
+ ok(ret == 0,
+ "process 2 lock ofs=1 len=1: %d - %s",
+ ret, strerror(errno));
+ diag("process 2 got read lock on 1,1\n");
+ ret = raw_fcntl_unlock(fd, 0, 2);
+ ok(ret == 0,
+ "process 2 unlock ofs=0 len=2: %d - %s",
+ ret, strerror(errno));
+ diag("process 2 released read lock on 0,2\n");
+ sleep(1);
+ send_char('d');
+ }
+
+ if (process == 1) {
+ expect_char('d');
+ }
+
+ diag("process %d has got to the end\n", process);
+
+ return 0;
+}
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 987d78c0b8b..09bc0a3192c 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -41,6 +41,7 @@ tdb1_unit_tests = [
'run-traverse-in-transaction',
'run-wronghash-fail',
'run-zero-append',
+ 'run-fcntl-deadlock',
'run-marklock-deadlock',
'run-allrecord-traverse-deadlock',
'run-mutex-openflags2',
--
2.11.0
From d0b7afee8b9bc72e9787783fc6dccf00cd1975c7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 31 May 2017 12:22:28 +1200
Subject: [PATCH 06/15] dsdb: Correctly call ldb_module_done in
dsdb_notification
If we just call ldb_request_done() then we never call the callback.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/dsdb_notification.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/dsdb_notification.c b/source4/dsdb/samdb/ldb_modules/dsdb_notification.c
index 19ae9dbdf53..ef92eac79d5 100644
--- a/source4/dsdb/samdb/ldb_modules/dsdb_notification.c
+++ b/source4/dsdb/samdb/ldb_modules/dsdb_notification.c
@@ -179,7 +179,7 @@ static int dsdb_notification_filter_search(struct ldb_module *module,
* It's the first time, let the caller comeback later
* as we won't find any new objects.
*/
- return ldb_request_done(req, LDB_SUCCESS);
+ return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
}
down_tree = talloc_zero(req, struct ldb_parse_tree);
--
2.11.0
From 195bbfd2049c7c2c924e4a3026c5365aa173ccfd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 31 May 2017 10:44:34 +1200
Subject: [PATCH 07/15] ldb: Rename module -> next_module for clarity
This helps make some future commits less confusing
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
lib/ldb/common/ldb.c | 80 +++++++++++++++++++++++++++-------------------------
1 file changed, 41 insertions(+), 39 deletions(-)
diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index a3a00028711..c9503303331 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -326,17 +326,19 @@ int ldb_error_at(struct ldb_context *ldb, int ecode,
#define FIRST_OP_NOERR(ldb, op) do { \
- module = ldb->modules; \
- while (module && module->ops->op == NULL) module = module->next; \
- if ((ldb->flags & LDB_FLG_ENABLE_TRACING) && module) { \
+ next_module = ldb->modules; \
+ while (next_module && next_module->ops->op == NULL) { \
+ next_module = next_module->next; \
+ }; \
+ if ((ldb->flags & LDB_FLG_ENABLE_TRACING) && next_module) { \
ldb_debug(ldb, LDB_DEBUG_TRACE, "ldb_trace_request: (%s)->" #op, \
- module->ops->name); \
+ next_module->ops->name); \
} \
} while (0)
#define FIRST_OP(ldb, op) do { \
FIRST_OP_NOERR(ldb, op); \
- if (module == NULL) { \
+ if (next_module == NULL) { \
ldb_asprintf_errstring(ldb, "unable to find module or backend to handle operation: " #op); \
return LDB_ERR_OPERATIONS_ERROR; \
} \
@@ -348,7 +350,7 @@ int ldb_error_at(struct ldb_context *ldb, int ecode,
*/
int ldb_transaction_start(struct ldb_context *ldb)
{
- struct ldb_module *module;
+ struct ldb_module *next_module;
int status;
ldb_debug(ldb, LDB_DEBUG_TRACE,
@@ -369,7 +371,7 @@ int ldb_transaction_start(struct ldb_context *ldb)
ldb_reset_err_string(ldb);
- status = module->ops->start_transaction(module);
+ status = next_module->ops->start_transaction(next_module);
if (status != LDB_SUCCESS) {
if (ldb->err_string == NULL) {
/* no error string was setup by the backend */
@@ -378,13 +380,13 @@ int ldb_transaction_start(struct ldb_context *ldb)
ldb_strerror(status),
status);
}
- if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
- ldb_debug(module->ldb, LDB_DEBUG_TRACE, "start ldb transaction error: %s",
- ldb_errstring(module->ldb));
+ if ((next_module && next_module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(next_module->ldb, LDB_DEBUG_TRACE, "start ldb transaction error: %s",
+ ldb_errstring(next_module->ldb));
}
} else {
- if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
- ldb_debug(module->ldb, LDB_DEBUG_TRACE, "start ldb transaction success");
+ if ((next_module && next_module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(next_module->ldb, LDB_DEBUG_TRACE, "start ldb transaction success");
}
}
return status;
@@ -395,7 +397,7 @@ int ldb_transaction_start(struct ldb_context *ldb)
*/
int ldb_transaction_prepare_commit(struct ldb_context *ldb)
{
- struct ldb_module *module;
+ struct ldb_module *next_module;
int status;
if (ldb->prepare_commit_done) {
@@ -418,17 +420,17 @@ int ldb_transaction_prepare_commit(struct ldb_context *ldb)
/* call prepare transaction if available */
FIRST_OP_NOERR(ldb, prepare_commit);
- if (module == NULL) {
+ if (next_module == NULL) {
return LDB_SUCCESS;
}
- status = module->ops->prepare_commit(module);
+ status = next_module->ops->prepare_commit(next_module);
if (status != LDB_SUCCESS) {
ldb->transaction_active--;
- /* if a module fails the prepare then we need
+ /* if a next_module fails the prepare then we need
to call the end transaction for everyone */
FIRST_OP(ldb, del_transaction);
- module->ops->del_transaction(module);
+ next_module->ops->del_transaction(next_module);
if (ldb->err_string == NULL) {
/* no error string was setup by the backend */
ldb_asprintf_errstring(ldb,
@@ -436,9 +438,9 @@ int ldb_transaction_prepare_commit(struct ldb_context *ldb)
ldb_strerror(status),
status);
}
- if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
- ldb_debug(module->ldb, LDB_DEBUG_TRACE, "prepare commit transaction error: %s",
- ldb_errstring(module->ldb));
+ if ((next_module && next_module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(next_module->ldb, LDB_DEBUG_TRACE, "prepare commit transaction error: %s",
+ ldb_errstring(next_module->ldb));
}
}
@@ -451,7 +453,7 @@ int ldb_transaction_prepare_commit(struct ldb_context *ldb)
*/
int ldb_transaction_commit(struct ldb_context *ldb)
{
- struct ldb_module *module;
+ struct ldb_module *next_module;
int status;
status = ldb_transaction_prepare_commit(ldb);
@@ -480,7 +482,7 @@ int ldb_transaction_commit(struct ldb_context *ldb)
ldb_reset_err_string(ldb);
FIRST_OP(ldb, end_transaction);
- status = module->ops->end_transaction(module);
+ status = next_module->ops->end_transaction(next_module);
if (status != LDB_SUCCESS) {
if (ldb->err_string == NULL) {
/* no error string was setup by the backend */
@@ -489,13 +491,13 @@ int ldb_transaction_commit(struct ldb_context *ldb)
ldb_strerror(status),
status);
}
- if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
- ldb_debug(module->ldb, LDB_DEBUG_TRACE, "commit ldb transaction error: %s",
- ldb_errstring(module->ldb));
+ if ((next_module && next_module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(next_module->ldb, LDB_DEBUG_TRACE, "commit ldb transaction error: %s",
+ ldb_errstring(next_module->ldb));
}
/* cancel the transaction */
FIRST_OP(ldb, del_transaction);
- module->ops->del_transaction(module);
+ next_module->ops->del_transaction(next_module);
}
return status;
}
@@ -506,7 +508,7 @@ int ldb_transaction_commit(struct ldb_context *ldb)
*/
int ldb_transaction_cancel(struct ldb_context *ldb)
{
- struct ldb_module *module;
+ struct ldb_module *next_module;
int status;
ldb->transaction_active--;
@@ -529,7 +531,7 @@ int ldb_transaction_cancel(struct ldb_context *ldb)
FIRST_OP(ldb, del_transaction);
- status = module->ops->del_transaction(module);
+ status = next_module->ops->del_transaction(next_module);
if (status != LDB_SUCCESS) {
if (ldb->err_string == NULL) {
/* no error string was setup by the backend */
@@ -538,9 +540,9 @@ int ldb_transaction_cancel(struct ldb_context *ldb)
ldb_strerror(status),
status);
}
- if ((module && module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
- ldb_debug(module->ldb, LDB_DEBUG_TRACE, "cancel ldb transaction error: %s",
- ldb_errstring(module->ldb));
+ if ((next_module && next_module->ldb->flags & LDB_FLG_ENABLE_TRACING)) {
+ ldb_debug(next_module->ldb, LDB_DEBUG_TRACE, "cancel ldb transaction error: %s",
+ ldb_errstring(next_module->ldb));
}
}
return status;
@@ -972,7 +974,7 @@ static int ldb_msg_check_element_flags(struct ldb_context *ldb,
*/
int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
{
- struct ldb_module *module;
+ struct ldb_module *next_module;
int ret;
if (req->callback == NULL) {
@@ -996,7 +998,7 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
return LDB_ERR_INVALID_DN_SYNTAX;
}
FIRST_OP(ldb, search);
- ret = module->ops->search(module, req);
+ ret = next_module->ops->search(next_module, req);
break;
case LDB_ADD:
if (!ldb_dn_validate(req->op.add.message->dn)) {
@@ -1024,7 +1026,7 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
*/
return ret;
}
- ret = module->ops->add(module, req);
+ ret = next_module->ops->add(next_module, req);
break;
case LDB_MODIFY:
if (!ldb_dn_validate(req->op.mod.message->dn)) {
@@ -1041,7 +1043,7 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
*/
return ret;
}
- ret = module->ops->modify(module, req);
+ ret = next_module->ops->modify(next_module, req);
break;
case LDB_DELETE:
if (!ldb_dn_validate(req->op.del.dn)) {
@@ -1050,7 +1052,7 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
return LDB_ERR_INVALID_DN_SYNTAX;
}
FIRST_OP(ldb, del);
- ret = module->ops->del(module, req);
+ ret = next_module->ops->del(next_module, req);
break;
case LDB_RENAME:
if (!ldb_dn_validate(req->op.rename.olddn)) {
@@ -1064,15 +1066,15 @@ int ldb_request(struct ldb_context *ldb, struct ldb_request *req)
return LDB_ERR_INVALID_DN_SYNTAX;
}
FIRST_OP(ldb, rename);
- ret = module->ops->rename(module, req);
+ ret = next_module->ops->rename(next_module, req);
break;
case LDB_EXTENDED:
FIRST_OP(ldb, extended);
- ret = module->ops->extended(module, req);
+ ret = next_module->ops->extended(next_module, req);
break;
default:
FIRST_OP(ldb, request);
- ret = module->ops->request(module, req);
+ ret = next_module->ops->request(next_module, req);
break;
}
--
2.11.0
From e8fec130488e5a36d3338b51d8fcb5b67c0b2888 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 7 Jun 2017 11:47:15 +1200
Subject: [PATCH 08/15] selftest: Add a test for @ATTRIBUTES and @INDEXLIST
generation
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/dsdb_schema_attributes.py | 138 +++++++++++++++++++++++++++
source4/selftest/tests.py | 3 +
source4/setup/schema_samba4.ldif | 1 +
3 files changed, 142 insertions(+)
create mode 100644 python/samba/tests/dsdb_schema_attributes.py
diff --git a/python/samba/tests/dsdb_schema_attributes.py b/python/samba/tests/dsdb_schema_attributes.py
new file mode 100644
index 00000000000..28f9078a5b9
--- /dev/null
+++ b/python/samba/tests/dsdb_schema_attributes.py
@@ -0,0 +1,138 @@
+# -*- coding: utf-8 -*-
+#
+# Unix SMB/CIFS implementation.
+# Copyright (C) Kamen Mazdrashki <kamenim at samba.org> 2010
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+#
+# Usage:
+# export SUBUNITRUN=$samba4srcdir/scripting/bin/subunitrun
+# PYTHONPATH="$PYTHONPATH:$samba4srcdir/dsdb/tests/python" $SUBUNITRUN dsdb_schema_attributes
+#
+
+import sys
+import time
+import random
+
+import samba.tests
+import ldb
+from ldb import SCOPE_BASE, LdbError
+
+import samba.tests
+
+class SchemaAttributesTestCase(samba.tests.TestCase):
+
+ def setUp(self):
+ super(SchemaAttributesTestCase, self).setUp()
+
+ self.lp = samba.tests.env_loadparm()
+ self.samdb = samba.tests.connect_samdb(self.lp.samdb_url())
+
+ # fetch rootDSE
+ res = self.samdb.search(base="", expression="", scope=SCOPE_BASE, attrs=["*"])
+ self.assertEquals(len(res), 1)
+ self.schema_dn = res[0]["schemaNamingContext"][0]
+ self.base_dn = res[0]["defaultNamingContext"][0]
+ self.forest_level = int(res[0]["forestFunctionality"][0])
+
+ def tearDown(self):
+ super(SchemaAttributesTestCase, self).tearDown()
+
+ def _ldap_schemaUpdateNow(self):
+ ldif = """
+dn:
+changetype: modify
+add: schemaUpdateNow
+schemaUpdateNow: 1
+"""
+ self.samdb.modify_ldif(ldif)
+
+ def _make_obj_names(self, prefix):
+ obj_name = prefix + time.strftime("%s", time.gmtime())
+ obj_ldap_name = obj_name.replace("-", "")
+ obj_dn = "CN=%s,%s" % (obj_name, self.schema_dn)
+ return (obj_name, obj_ldap_name, obj_dn)
+
+ def _make_attr_ldif(self, attr_name, attr_dn, sub_oid, extra=None):
+ ldif = """
+dn: """ + attr_dn + """
+objectClass: top
+objectClass: attributeSchema
+adminDescription: """ + attr_name + """
+adminDisplayName: """ + attr_name + """
+cn: """ + attr_name + """
+attributeId: 1.3.6.1.4.1.7165.4.6.1.8.%d.""" % sub_oid + str(random.randint(1,100000)) + """
+attributeSyntax: 2.5.5.12
+omSyntax: 64
+instanceType: 4
+isSingleValued: TRUE
+systemOnly: FALSE
+"""
+
+ if extra is not None:
+ ldif += extra + "\n"
+
+ return ldif
+
+ def test_AddIndexedAttribute(self):
+ # create names for an attribute to add
+ (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-Attr-")
+ ldif = self._make_attr_ldif(attr_name, attr_dn, 1,
+ "searchFlags: %d" % samba.dsdb.SEARCH_FLAG_ATTINDEX)
+
+ # add the new attribute
+ self.samdb.add_ldif(ldif)
+ self._ldap_schemaUpdateNow()
+
+ # Check @ATTRIBUTES
+
+ attr_res = self.samdb.search(base="@ATTRIBUTES", scope=ldb.SCOPE_BASE)
+
+ self.assertIn(attr_ldap_name, attr_res[0])
+ self.assertEquals(len(attr_res[0][attr_ldap_name]), 1)
+ self.assertEquals(attr_res[0][attr_ldap_name][0], "CASE_INSENSITIVE")
+
+ # Check @INDEXLIST
+
+ idx_res = self.samdb.search(base="@INDEXLIST", scope=ldb.SCOPE_BASE)
+
+ self.assertIn(attr_ldap_name, [str(x) for x in idx_res[0]["@IDXATTR"]])
+
+
+
+ def test_AddUnIndexedAttribute(self):
+
+ # create names for an attribute to add
+ (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-Attr-")
+ ldif = self._make_attr_ldif(attr_name, attr_dn, 2)
+
+ # add the new attribute
+ self.samdb.add_ldif(ldif)
+ self._ldap_schemaUpdateNow()
+
+ # Check @ATTRIBUTES
+
+ attr_res = self.samdb.search(base="@ATTRIBUTES", scope=ldb.SCOPE_BASE)
+
+ self.assertIn(attr_ldap_name, attr_res[0])
+ self.assertEquals(len(attr_res[0][attr_ldap_name]), 1)
+ self.assertEquals(attr_res[0][attr_ldap_name][0], "CASE_INSENSITIVE")
+
+ # Check @INDEXLIST
+
+ idx_res = self.samdb.search(base="@INDEXLIST", scope=ldb.SCOPE_BASE)
+
+ self.assertNotIn(attr_ldap_name, [str(x) for x in idx_res[0]["@IDXATTR"]])
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 43c218de87e..f61c93f0f4e 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -668,6 +668,9 @@ planoldpythontestsuite("ad_dc_ntvfs", "dsdb_schema_info",
extra_path=[os.path.join(samba4srcdir, 'dsdb/tests/python')],
name="samba4.schemaInfo.python(ad_dc_ntvfs)",
extra_args=['-U"$DOMAIN/$DC_USERNAME%$DC_PASSWORD"'])
+
+planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dsdb_schema_attributes")
+
plantestsuite_loadlist("samba4.urgent_replication.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(samba4srcdir, "dsdb/tests/python/urgent_replication.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.dirsync.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/dirsync.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
index d8bc2d994c3..fcfaf9819b7 100644
--- a/source4/setup/schema_samba4.ldif
+++ b/source4/setup/schema_samba4.ldif
@@ -21,6 +21,7 @@
## 1.3.6.1.4.1.7165.4.6.1.5.x - repl_schema.py
## 1.3.6.1.4.1.7165.4.6.1.6.x - ldap_schema.py
## 1.3.6.1.4.1.7165.4.6.1.7.x - dsdb_schema_info.py
+## 1.3.6.1.4.1.7165.4.6.1.8.x - dsdb_schema_attributes.py
## 1.3.6.1.4.1.7165.4.6.2.x - SELFTEST random classes
## 1.3.6.1.4.1.7165.4.6.2.1.x - ldap_syntaxes.py
--
2.11.0
From eec3a22605c3fa7adf79872a40669bcf1c790cd7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 12 Jun 2017 14:12:53 +1200
Subject: [PATCH 09/15] selftest: Add pygensec tests for GSS-SPNEGO and Win2000
emulated SPNEGO
This is to provide some unit testing coverage for these different modes of operation
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/gensec.py | 61 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 6 deletions(-)
diff --git a/python/samba/tests/gensec.py b/python/samba/tests/gensec.py
index 368d406b6e3..fe87aa22222 100644
--- a/python/samba/tests/gensec.py
+++ b/python/samba/tests/gensec.py
@@ -32,6 +32,8 @@ class GensecTests(samba.tests.TestCase):
self.settings = {}
self.settings["lp_ctx"] = self.lp_ctx = samba.tests.env_loadparm()
self.settings["target_hostname"] = self.lp_ctx.get("netbios name")
+ self.lp_ctx.set("spnego:simulate_w2k", "no")
+
"""This is just for the API tests"""
self.gensec = gensec.Security.start_client(self.settings)
@@ -44,7 +46,7 @@ class GensecTests(samba.tests.TestCase):
def test_info_uninitialized(self):
self.assertRaises(RuntimeError, self.gensec.session_info)
- def test_update(self):
+ def _test_update(self, mech):
"""Test GENSEC by doing an exchange with ourselves using GSSAPI against a KDC"""
"""Start up a client and server GENSEC instance to test things with"""
@@ -52,7 +54,7 @@ class GensecTests(samba.tests.TestCase):
self.gensec_client = gensec.Security.start_client(self.settings)
self.gensec_client.set_credentials(self.get_credentials())
self.gensec_client.want_feature(gensec.FEATURE_SEAL)
- self.gensec_client.start_mech_by_sasl_name("GSSAPI")
+ self.gensec_client.start_mech_by_sasl_name(mech)
self.gensec_server = gensec.Security.start_server(settings=self.settings,
auth_context=auth.AuthContext(lp_ctx=self.lp_ctx))
@@ -62,25 +64,37 @@ class GensecTests(samba.tests.TestCase):
self.gensec_server.set_credentials(creds)
self.gensec_server.want_feature(gensec.FEATURE_SEAL)
- self.gensec_server.start_mech_by_sasl_name("GSSAPI")
+ self.gensec_server.start_mech_by_sasl_name(mech)
client_finished = False
server_finished = False
server_to_client = b""
+ client_to_server = b""
"""Run the actual call loop"""
- while not client_finished and not server_finished:
+ while True:
if not client_finished:
print("running client gensec_update")
(client_finished, client_to_server) = self.gensec_client.update(server_to_client)
if not server_finished:
print("running server gensec_update")
(server_finished, server_to_client) = self.gensec_server.update(client_to_server)
+
+ if client_finished and server_finished:
+ break
+
+ self.assertTrue(server_finished)
+ self.assertTrue(client_finished)
+
session_info = self.gensec_server.session_info()
test_bytes = b"Hello Server"
- test_wrapped = self.gensec_client.wrap(test_bytes)
- test_unwrapped = self.gensec_server.unwrap(test_wrapped)
+ try:
+ test_wrapped = self.gensec_client.wrap(test_bytes)
+ test_unwrapped = self.gensec_server.unwrap(test_wrapped)
+ except samba.NTSTATUSError as e:
+ self.fail(str(e))
+
self.assertEqual(test_bytes, test_unwrapped)
test_bytes = b"Hello Client"
test_wrapped = self.gensec_server.wrap(test_bytes)
@@ -91,6 +105,41 @@ class GensecTests(samba.tests.TestCase):
server_session_key = self.gensec_server.session_key()
self.assertEqual(client_session_key, server_session_key)
+ def test_update(self):
+ self._test_update("GSSAPI")
+
+ def test_update_spnego(self):
+ self._test_update("GSS-SPNEGO")
+
+ def test_update_w2k_spnego_client(self):
+ self.lp_ctx.set("spnego:simulate_w2k", "yes")
+
+ # Re-start the client with this set
+ self.gensec = gensec.Security.start_client(self.settings)
+
+ # Unset it for the server
+ self.lp_ctx.set("spnego:simulate_w2k", "no")
+
+ self._test_update("GSS-SPNEGO")
+
+ def test_update_w2k_spnego_server(self):
+ # Re-start the client with this set
+ self.gensec = gensec.Security.start_client(self.settings)
+
+ # Unset it for the server
+ self.lp_ctx.set("spnego:simulate_w2k", "yes")
+
+ self._test_update("GSS-SPNEGO")
+
+ def test_update_w2k_spnego(self):
+ self.lp_ctx.set("spnego:simulate_w2k", "no")
+
+ # Re-start the client with this set
+ self.gensec = gensec.Security.start_client(self.settings)
+
+ self._test_update("GSS-SPNEGO")
+
+
def test_max_update_size(self):
"""Test GENSEC by doing an exchange with ourselves using GSSAPI against a KDC"""
--
2.11.0
From c4c82e212ae44b9b04324080bf33a082cc2c44e2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 12 Jun 2017 14:27:53 +1200
Subject: [PATCH 10/15] selftest: Add test for gss_krb5/ntlmssp -> SPNEGO
These bare mechs are permitted to go direct to SPNEGO, which must cope with them
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/gensec.py | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/python/samba/tests/gensec.py b/python/samba/tests/gensec.py
index fe87aa22222..9cb6eea93ab 100644
--- a/python/samba/tests/gensec.py
+++ b/python/samba/tests/gensec.py
@@ -46,7 +46,7 @@ class GensecTests(samba.tests.TestCase):
def test_info_uninitialized(self):
self.assertRaises(RuntimeError, self.gensec.session_info)
- def _test_update(self, mech):
+ def _test_update(self, mech, client_mech=None):
"""Test GENSEC by doing an exchange with ourselves using GSSAPI against a KDC"""
"""Start up a client and server GENSEC instance to test things with"""
@@ -54,7 +54,10 @@ class GensecTests(samba.tests.TestCase):
self.gensec_client = gensec.Security.start_client(self.settings)
self.gensec_client.set_credentials(self.get_credentials())
self.gensec_client.want_feature(gensec.FEATURE_SEAL)
- self.gensec_client.start_mech_by_sasl_name(mech)
+ if client_mech is not None:
+ self.gensec_client.start_mech_by_name(client_mech)
+ else:
+ self.gensec_client.start_mech_by_sasl_name(mech)
self.gensec_server = gensec.Security.start_server(settings=self.settings,
auth_context=auth.AuthContext(lp_ctx=self.lp_ctx))
@@ -139,6 +142,12 @@ class GensecTests(samba.tests.TestCase):
self._test_update("GSS-SPNEGO")
+ def test_update_gss_krb5_to_spnego(self):
+ self._test_update("GSS-SPNEGO", "gssapi_krb5")
+
+ def test_update_ntlmssp_to_spnego(self):
+ self._test_update("GSS-SPNEGO", "ntlmssp")
+
def test_max_update_size(self):
"""Test GENSEC by doing an exchange with ourselves using GSSAPI against a KDC"""
--
2.11.0
From c307f48e3a89c08252506e36e05d973bae10f673 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Fri, 9 Jun 2017 14:13:25 +1200
Subject: [PATCH 11/15] stream_terminate_connection: Prevent use-after-free
This sometimes would show up as corrupted bytes during logs. Hammering
the LDAP server enough times managed to trigger an outright segfault.
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
source4/smbd/service_stream.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c
index bda28ad26f8..917a1876e07 100644
--- a/source4/smbd/service_stream.c
+++ b/source4/smbd/service_stream.c
@@ -55,6 +55,7 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char
struct tevent_context *event_ctx = srv_conn->event.ctx;
const struct model_ops *model_ops = srv_conn->model_ops;
struct loadparm_context *lp_ctx = srv_conn->lp_ctx;
+ TALLOC_CTX *frame = NULL;
if (!reason) reason = "unknown reason";
@@ -77,11 +78,20 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char
return;
}
+ frame = talloc_stackframe();
+
+ reason = talloc_strdup(frame, reason);
+ if (reason == NULL) {
+ reason = "OOM - unknown reason";
+ }
+
talloc_free(srv_conn->event.fde);
srv_conn->event.fde = NULL;
imessaging_cleanup(srv_conn->msg_ctx);
TALLOC_FREE(srv_conn);
model_ops->terminate(event_ctx, lp_ctx, reason);
+
+ TALLOC_FREE(frame);
}
/**
--
2.11.0
From 9bc0551756313cd57ff6f86ffb90c7ac04cd4bc5 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Jun 2017 11:20:58 +1200
Subject: [PATCH 12/15] selftest: Pass the dcerpc binding object to
self.waitForMessages in auth_log
This ensures that object is not cleaned up, triggering a disconnect before we get back
the audit messages. Otherwise they can be lost when the server task calls exit()
while the message thread is still trying to send them.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/tests/auth_log.py | 14 +++++++-------
python/samba/tests/auth_log_samlogon.py | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/python/samba/tests/auth_log.py b/python/samba/tests/auth_log.py
index 6f32712c339..ff84befda45 100644
--- a/python/samba/tests/auth_log.py
+++ b/python/samba/tests/auth_log.py
@@ -250,16 +250,16 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase):
binding = "[%s]" % binding
if service == "dnsserver":
- dnsserver.dnsserver("ncacn_ip_tcp:%s%s" % (self.server, binding),
- self.get_loadparm(),
- creds)
+ conn = dnsserver.dnsserver("ncacn_ip_tcp:%s%s" % (self.server, binding),
+ self.get_loadparm(),
+ creds)
elif service == "srvsvc":
- srvsvc.srvsvc("ncacn_ip_tcp:%s%s" % (self.server, binding),
- self.get_loadparm(),
- creds)
+ conn = srvsvc.srvsvc("ncacn_ip_tcp:%s%s" % (self.server, binding),
+ self.get_loadparm(),
+ creds)
- messages = self.waitForMessages(isLastExpectedMessage)
+ messages = self.waitForMessages(isLastExpectedMessage, conn)
checkFunction(messages, authTypes, service, binding, protection)
def rpc_ncacn_ip_tcp_ntlm_check(self, messages, authTypes, service,
diff --git a/python/samba/tests/auth_log_samlogon.py b/python/samba/tests/auth_log_samlogon.py
index cbef5a19308..d24986b68a5 100644
--- a/python/samba/tests/auth_log_samlogon.py
+++ b/python/samba/tests/auth_log_samlogon.py
@@ -152,7 +152,7 @@ class AuthLogTestsSamLogon(samba.tests.auth_log_base.AuthLogTestBase):
(validation, authoritative, netr_flags_out) = result
- messages = self.waitForMessages(isLastExpectedMessage)
+ messages = self.waitForMessages(isLastExpectedMessage, netlogon_conn)
checkFunction(messages)
def samlogon_check(self, messages):
--
2.11.0
From 5c33553c83a78c97982890808586b8c3c7010df2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 14 Jun 2017 13:12:32 +1200
Subject: [PATCH 13/15] dsdb: Ensure replication of renames works in schema
partition
This caused failures against vampire_dc (on large-dc), likely due to
more frequent replication propagating the record before it was renamed.
The DC ran out of RIDs and RID allocation causes schema replication,
which failed.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12841
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/common/util.c | 13 +++++++++++
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 9 ++++++-
source4/dsdb/samdb/ldb_modules/util.h | 1 +
source4/torture/drs/python/repl_schema.py | 31 +++++++++++++++++++++++++
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 8f74b45a84e..b46a7fc4054 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -49,6 +49,12 @@
#include "lib/util/access.h"
/*
+ * This included to allow us to handle DSDB_FLAG_REPLICATED_UPDATE in
+ * dsdb_request_add_controls()
+ */
+#include "dsdb/samdb/ldb_modules/util.h"
+
+/*
search the sam for the specified attributes in a specific domain, filter on
objectSid being in domain_sid.
*/
@@ -4309,6 +4315,13 @@ int dsdb_request_add_controls(struct ldb_request *req, uint32_t dsdb_flags)
}
}
+ if (dsdb_flags & DSDB_FLAG_REPLICATED_UPDATE) {
+ ret = ldb_request_add_control(req, DSDB_CONTROL_REPLICATED_UPDATE_OID, false, NULL);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+ }
+
return LDB_SUCCESS;
}
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 4f077c01a26..ce06ee89013 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -4519,7 +4519,14 @@ static int replmd_name_modify(struct replmd_replicated_request *ar,
goto failed;
}
- ret = dsdb_module_modify(ar->module, msg, DSDB_FLAG_OWN_MODULE, req);
+ /*
+ * We have to mark this as a replicated update otherwise
+ * schema_data may reject a rename in the schema partition
+ */
+
+ ret = dsdb_module_modify(ar->module, msg,
+ DSDB_FLAG_OWN_MODULE|DSDB_FLAG_REPLICATED_UPDATE,
+ req);
if (ret != LDB_SUCCESS) {
DEBUG(0,(__location__ ": Failed to modify rDN/name of conflict DN '%s' - %s",
ldb_dn_get_linearized(dn),
diff --git a/source4/dsdb/samdb/ldb_modules/util.h b/source4/dsdb/samdb/ldb_modules/util.h
index e40730557a3..5ecf0eee0d2 100644
--- a/source4/dsdb/samdb/ldb_modules/util.h
+++ b/source4/dsdb/samdb/ldb_modules/util.h
@@ -38,3 +38,4 @@ struct netlogon_samlogon_response;
#define DSDB_FLAG_OWN_MODULE 0x00400000
#define DSDB_FLAG_TOP_MODULE 0x00800000
#define DSDB_FLAG_TRUSTED 0x01000000
+#define DSDB_FLAG_REPLICATED_UPDATE 0x02000000
diff --git a/source4/torture/drs/python/repl_schema.py b/source4/torture/drs/python/repl_schema.py
index 0ce70e75282..4514237cc55 100644
--- a/source4/torture/drs/python/repl_schema.py
+++ b/source4/torture/drs/python/repl_schema.py
@@ -417,3 +417,34 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase):
nc_dn=self.schema_dn, forced=True)
self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1,
nc_dn=self.domain_dn, forced=True)
+
+ def test_rename(self):
+ """Basic plan is to create a classSchema
+ and attributeSchema objects, replicate Schema NC
+ and then check all objects are replicated correctly"""
+
+ # add new classSchema object
+ (c_ldn, c_dn) = self._schema_new_class(self.ldb_dc1, "cls-B", 20)
+
+ self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1,
+ nc_dn=self.schema_dn, forced=True)
+
+ # check objects are replicated
+ self._check_object(c_dn)
+
+ # rename the Class CN
+ c_dn_new = ldb.Dn(self.ldb_dc1, str(c_dn))
+ c_dn_new.set_component(0,
+ "CN",
+ c_dn.get_component_value(0) + "-NEW")
+ try:
+ self.ldb_dc1.rename(c_dn, c_dn_new)
+ except LdbError, (num, _):
+ self.fail("failed to change CN for %s: %s" % (c_dn, _))
+
+ # force replication from DC1 to DC2
+ self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1,
+ nc_dn=self.schema_dn, forced=True)
+
+ # check objects are replicated
+ self._check_object(c_dn_new)
--
2.11.0
From 944021704ea2f5be672ed543499b7a188503bdd2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 14 Jun 2017 14:13:18 +1200
Subject: [PATCH 14/15] dsdb: Improve debug messages
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 4 ++--
source4/dsdb/samdb/ldb_modules/schema_data.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index ce06ee89013..2715db84083 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -4528,7 +4528,7 @@ static int replmd_name_modify(struct replmd_replicated_request *ar,
DSDB_FLAG_OWN_MODULE|DSDB_FLAG_REPLICATED_UPDATE,
req);
if (ret != LDB_SUCCESS) {
- DEBUG(0,(__location__ ": Failed to modify rDN/name of conflict DN '%s' - %s",
+ DEBUG(0,(__location__ ": Failed to modify rDN/name of DN being DRS renamed '%s' - %s",
ldb_dn_get_linearized(dn),
ldb_errstring(ldb_module_get_ctx(ar->module))));
return ret;
@@ -4540,7 +4540,7 @@ static int replmd_name_modify(struct replmd_replicated_request *ar,
failed:
talloc_free(msg);
- DEBUG(0,(__location__ ": Failed to setup modify rDN/name of conflict DN '%s'",
+ DEBUG(0,(__location__ ": Failed to setup modify rDN/name of DN being DRS renamed '%s'",
ldb_dn_get_linearized(dn)));
return LDB_ERR_OPERATIONS_ERROR;
}
diff --git a/source4/dsdb/samdb/ldb_modules/schema_data.c b/source4/dsdb/samdb/ldb_modules/schema_data.c
index 996b1f22386..5362ef091c3 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_data.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_data.c
@@ -171,13 +171,13 @@ static int schema_data_add(struct ldb_module *module, struct ldb_request *req)
if (!schema->fsmo.we_are_master && !rodc) {
ldb_debug_set(ldb, LDB_DEBUG_ERROR,
- "schema_data_add: we are not master: reject request\n");
+ "schema_data_add: we are not master: reject add request\n");
return LDB_ERR_UNWILLING_TO_PERFORM;
}
if (!schema->fsmo.update_allowed && !rodc) {
ldb_debug_set(ldb, LDB_DEBUG_ERROR,
- "schema_data_add: updates are not allowed: reject request\n");
+ "schema_data_add: updates are not allowed: reject add request\n");
return LDB_ERR_UNWILLING_TO_PERFORM;
}
@@ -333,13 +333,13 @@ static int schema_data_modify(struct ldb_module *module, struct ldb_request *req
if (!schema->fsmo.we_are_master && !rodc) {
ldb_debug_set(ldb, LDB_DEBUG_ERROR,
- "schema_data_modify: we are not master: reject request\n");
+ "schema_data_modify: we are not master: reject modify request\n");
return LDB_ERR_UNWILLING_TO_PERFORM;
}
if (!schema->fsmo.update_allowed && !rodc) {
ldb_debug_set(ldb, LDB_DEBUG_ERROR,
- "schema_data_modify: updates are not allowed: reject request\n");
+ "schema_data_modify: updates are not allowed: reject modify request\n");
return LDB_ERR_UNWILLING_TO_PERFORM;
}
--
2.11.0
From 9ea9c51efe8de321d8e7dc8f5fe8956f800d485c Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 16 Nov 2016 14:44:40 +1300
Subject: [PATCH 15/15] repl: Set GET_ALL_GROUP_MEMBERSHIP flag in the drepl
server
Although we do not currently support this in the server, this will cause
data loss against a Windows DC unless we set this flag as per the docs.
This flag is required for the RODC.
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/kcc/__init__.py | 1 -
source4/dsdb/kcc/kcc_periodic.c | 1 -
source4/dsdb/repl/drepl_out_helpers.c | 14 ++++++++++++++
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/python/samba/kcc/__init__.py b/python/samba/kcc/__init__.py
index ad322a5c542..f775a11b264 100644
--- a/python/samba/kcc/__init__.py
+++ b/python/samba/kcc/__init__.py
@@ -909,7 +909,6 @@ class KCC(object):
drsuapi.DRSUAPI_DRS_PER_SYNC |
drsuapi.DRSUAPI_DRS_ADD_REF |
drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING |
- drsuapi.DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP |
drsuapi.DRSUAPI_DRS_NONGC_RO_REP)
if t_repsFrom.replica_flags != replica_flags:
t_repsFrom.replica_flags = replica_flags
diff --git a/source4/dsdb/kcc/kcc_periodic.c b/source4/dsdb/kcc/kcc_periodic.c
index 8c4b70a1c94..fa19ba7efc5 100644
--- a/source4/dsdb/kcc/kcc_periodic.c
+++ b/source4/dsdb/kcc/kcc_periodic.c
@@ -178,7 +178,6 @@ uint32_t kccsrv_replica_flags(struct kccsrv_service *s)
DRSUAPI_DRS_PER_SYNC |
DRSUAPI_DRS_ADD_REF |
DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING |
- DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP |
DRSUAPI_DRS_NONGC_RO_REP;
}
return DRSUAPI_DRS_INIT_SYNC |
diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
index d526f4558a5..079edc8ba46 100644
--- a/source4/dsdb/repl/drepl_out_helpers.c
+++ b/source4/dsdb/repl/drepl_out_helpers.c
@@ -518,7 +518,21 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
} else {
replica_flags |= DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING;
}
+
+ /*
+ * As per MS-DRSR:
+ *
+ * 4.1.10.4
+ * Client Behavior When Sending the IDL_DRSGetNCChanges Request
+ *
+ * 4.1.10.4.1
+ * ReplicateNCRequestMsg
+ */
+ replica_flags |= DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP;
+ } else {
+ replica_flags |= DRSUAPI_DRS_GET_ALL_GROUP_MEMBERSHIP;
}
+
if (state->op->extended_op != DRSUAPI_EXOP_NONE) {
/*
* If it's an exop never set the ADD_REF even if it's in
--
2.11.0
-------------- next part --------------
From e99b454804e13c69cd89e60b6a2441ece5dbd099 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 87552d2b1e9d97472cb7a113745c0a4a8068cc5c 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 7d1edc34de1674fbc3c089422d7832c60e1e61bd 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 f0ac667a082235f9d842f2f873a13280544b6029 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 d933285b478dba6608b3a3cdd3b530156f54ed81 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 e69b91cb318e1edb2f6db76549a9c963bcfbe8ce 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 27d0e9477816f068b63a970a950322164b347957 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 07/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 | 272 +++++++++++++++++++++++++++++++++
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, 277 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..ef9c53e1a4e
--- /dev/null
+++ b/lib/ldb/ABI/ldb-1.1.31.sigs
@@ -0,0 +1,272 @@
+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_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 b24d5c7fde47195d04060ce5acd59a4e7f1e565e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 12 May 2017 01:35:13 +0200
Subject: [PATCH 08/18] dsdb: Allocate OIDs for DB read lock and unlock
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/samdb.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
index 5dce37e2e9c..471aeeaf443 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -299,6 +299,13 @@ struct dsdb_extended_allocate_rid {
};
/*
+ * these are declared in ldb_module.h and takes no data. This entry is
+ * included to avoid duplicate OID allocation.
+ */
+/* #define LDB_EXTENDED_READ_LOCK_DB "1.3.6.1.4.1.7165.4.4.10" */
+/* #define LDB_EXTENDED_READ_UNLOCK_DB "1.3.6.1.4.1.7165.4.4.11" */
+
+/*
* passed from the descriptor module in order to
* store the recalucated nTSecurityDescriptor without
* modifying the replPropertyMetaData.
--
2.11.0
From cdc458b65ff2847327ac785f84fae87e38412cb1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 12 May 2017 01:38:14 +0200
Subject: [PATCH 09/18] ldb: Define OIDs LDB_EXTENDED_READ_LOCK_DB and
LDB_EXTENDED_READ_UNLOCK_DB
These will be used to allow Samba to provide a consistent view of the DB
despite the use of multiple databases via the partitions module
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/include/ldb_module.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 3d56e68eddd..b6eec0d9719 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -508,4 +508,12 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
*/
void ldb_handle_use_global_event_context(struct ldb_handle *handle);
+/*
+ * These extended operations allow a read lock to be taken during the
+ * overall search operation, not just for each DB read. This ensures
+ * no modification while the search is being processed.
+ */
+#define LDB_EXTENDED_READ_LOCK_DB "1.3.6.1.4.1.7165.4.4.10"
+#define LDB_EXTENDED_READ_UNLOCK_DB "1.3.6.1.4.1.7165.4.4.11"
+
#endif
--
2.11.0
From ebf904c951a5f7b5f18fa553dddbb94c0bd08a92 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 24 May 2017 11:05:18 +1200
Subject: [PATCH 10/18] dsdb: Allow locking requests no matter who the caller
is
The LDB_EXTENDED_READ_LOCK_DB and LDB_EXTENDED_READ_UNLOCK_DB operations are not
exposed over LDAP
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/acl.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 27d4e7659cc..d710a65b8fb 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -1934,9 +1934,13 @@ static int acl_extended(struct ldb_module *module, struct ldb_request *req)
struct ldb_context *ldb = ldb_module_get_ctx(module);
struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID);
- /* allow everybody to read the sequence number */
- if (strcmp(req->op.extended.oid,
- LDB_EXTENDED_SEQUENCE_NUMBER) == 0) {
+ /* allow everybody to read the sequence number, lock and unlock the DB */
+ if ((strcmp(req->op.extended.oid,
+ LDB_EXTENDED_SEQUENCE_NUMBER) == 0) ||
+ (strcmp(req->op.extended.oid,
+ LDB_EXTENDED_READ_LOCK_DB) == 0) ||
+ (strcmp(req->op.extended.oid,
+ LDB_EXTENDED_READ_UNLOCK_DB) == 0)) {
return ldb_next_request(module, req);
}
--
2.11.0
From 761cf95f043c281aa6ecaa32e2343c4b854653dd 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 11/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 722b48e3301a63c8902e0a99b560dee4f2ac05ad 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 | 147 ++++++++++++++++++++++++++++-
source4/dsdb/samdb/ldb_modules/partition.h | 1 +
2 files changed, 147 insertions(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 08a959cabb5..f92552feb68 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,143 @@ 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_backend(struct ldb_module *module, struct ldb_request *req, bool lock)
+{
+ 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);
+ /* Look at base DN */
+ /* Figure out which partition it is under */
+ /* Skip the lot if 'data' isn't here yet (initialization) */
+ if (ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING) {
+ if (lock) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE, "partition_lock_backend() -> (metadata partition)");
+ } else {
+ ldb_debug(ldb, LDB_DEBUG_TRACE, "partition_unlock_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
+ */
+ if (lock) {
+ /*
+ * 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_request(module, req);
+ if (ret != LDB_SUCCESS) {
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to %s db: %s / %s for metadata partition",
+ lock ? "lock" : "unlock",
+ ldb_errstring(ldb),
+ ldb_strerror(ret));
+
+ if (lock == true) {
+ return ret;
+ }
+ }
+
+ for (i=0; data && data->partitions && data->partitions[i]; i++) {
+ struct ldb_request *unlock_req = NULL;
+ if ((module && ldb_module_flags(ldb) & LDB_FLG_ENABLE_TRACING)) {
+ if (lock) {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_lock_backend() -> %s",
+ ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+ } else {
+ ldb_debug(ldb, LDB_DEBUG_TRACE,
+ "partition_unlock_backend() -> %s",
+ ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+ }
+ }
+ ret = partition_request(data->partitions[i]->module, req);
+ if (ret == LDB_SUCCESS) {
+ continue;
+ }
+
+ ldb_debug_set(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to %s db: %s / %s for %s",
+ lock ? "lock" : "unlock",
+ ldb_errstring(ldb),
+ ldb_strerror(ret),
+ ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
+
+ if (lock == false) {
+ /*
+ * On unlock failure, still try unlocking the
+ * rest.
+ */
+ continue;
+ }
+
+ ret2 = ldb_build_extended_req(&unlock_req, ldb, req,
+ LDB_EXTENDED_READ_UNLOCK_DB, NULL, NULL,
+ NULL, NULL,
+ NULL);
+ LDB_REQ_SET_LOCATION(unlock_req);
+ if (ret2 != LDB_SUCCESS) {
+ /* Really, what else can we do? */
+ ldb_debug(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to allocate memory to unlock db");
+ return ret;
+ }
+ /* Back it out, if it fails on one */
+ for (i--; i >= 0; i--) {
+ ret2 = partition_request(data->partitions[i]->module, unlock_req);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret2));
+ }
+ }
+ ret2 = partition_request(module, unlock_req);
+ if (ret2 != LDB_SUCCESS) {
+ ldb_debug(ldb,
+ LDB_DEBUG_FATAL,
+ "Failed to unlock db: %s / %s",
+ ldb_errstring(ldb),
+ ldb_strerror(ret2));
+ }
+ return ret;
+ }
+
+ if (lock) {
+ data->db_read_locked++;
+ } else {
+ data->db_read_locked--;
+ }
+
+ return LDB_SUCCESS;
+}
+
+
/* extended */
static int partition_extended(struct ldb_module *module, struct ldb_request *req)
{
@@ -1157,6 +1294,14 @@ static int partition_extended(struct ldb_module *module, struct ldb_request *req
return ldb_next_request(module, req);
}
+ if (strcmp(req->op.extended.oid, LDB_EXTENDED_READ_LOCK_DB) == 0) {
+ return partition_lock_backend(module, req, true);
+ }
+
+ if (strcmp(req->op.extended.oid, LDB_EXTENDED_READ_UNLOCK_DB) == 0) {
+ return partition_lock_backend(module, req, false);
+ }
+
if (strcmp(req->op.extended.oid, DSDB_EXTENDED_SCHEMA_UPDATE_NOW_OID) == 0) {
/* Update the metadata.tdb to increment the schema version if needed*/
DEBUG(10, ("Incrementing the sequence_number after schema_update_now\n"));
diff --git a/source4/dsdb/samdb/ldb_modules/partition.h b/source4/dsdb/samdb/ldb_modules/partition.h
index ea05e9404d5..cd6cbdb700c 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.h
+++ b/source4/dsdb/samdb/ldb_modules/partition.h
@@ -55,6 +55,7 @@ struct partition_private_data {
uint64_t metadata_seq;
uint32_t in_transaction;
+ uint32_t db_read_locked;
struct ldb_message *forced_module_msg;
};
--
2.11.0
From f3581a5b46d18ccc5b61bba9a2f5e0fc9fdd4a95 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 13/18] ldb_tdb: Implement OIDs LDB_EXTENDED_READ_LOCK_DB and
LDB_EXTENDED_READ_UNLOCK_DB
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 | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 5c6d3f5544b..65465ec4807 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1521,6 +1521,33 @@ static int ltdb_handle_request(struct ldb_module *module,
return LDB_SUCCESS;
}
+/*
+ * lock and unlock must be handled without an event loop
+ *
+ * This means we have to handle it here, before the ltdb_handle_request()
+ */
+static int ltdb_handle_extended_or_lock(struct ldb_module *module,
+ struct ldb_request *req)
+{
+ if (strcmp(req->op.extended.oid,
+ LDB_EXTENDED_READ_LOCK_DB) == 0) {
+ /* Lock (or increment ref count) on the DB */
+// ldb_debug(ldb_module_get_ctx(module),
+// LDB_DEBUG_FATAL,
+// "lock db");
+ return ltdb_lock_read(module);
+ } else if (strcmp(req->op.extended.oid,
+ LDB_EXTENDED_READ_UNLOCK_DB) == 0) {
+ /* Unlock (or decrement ref count) on the DB */
+// ldb_debug(ldb_module_get_ctx(module),
+// LDB_DEBUG_FATAL,
+// "UNlock db");
+ return ltdb_unlock_read(module);
+ } else {
+ return ltdb_handle_request(module, req);
+ }
+}
+
static int ltdb_init_rootdse(struct ldb_module *module)
{
/* ignore errors on this - we expect it for non-sam databases */
@@ -1538,7 +1565,7 @@ static const struct ldb_module_ops ltdb_ops = {
.modify = ltdb_handle_request,
.del = ltdb_handle_request,
.rename = ltdb_handle_request,
- .extended = ltdb_handle_request,
+ .extended = ltdb_handle_extended_or_lock,
.start_transaction = ltdb_start_trans,
.end_transaction = ltdb_end_trans,
.prepare_commit = ltdb_prepare_commit,
--
2.11.0
From fbff5f63a471818652c7a0daadeeab4723a1bd9c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 23 May 2017 15:14:28 +1200
Subject: [PATCH 14/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 | 183 +++++++++++++++++++++++++++++++++++++++-
lib/ldb/include/ldb_module.h | 3 +
lib/ldb/tests/ldb_mod_op_test.c | 38 ++++++++-
3 files changed, 221 insertions(+), 3 deletions(-)
diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index c9503303331..9d892d9318e 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -966,6 +966,170 @@ static int ldb_msg_check_element_flags(struct ldb_context *ldb,
return LDB_SUCCESS;
}
+/*
+ * Because we choose to overload the extended operation API, we have
+ * to have this, even despite the locking calls never using callbacks
+ */
+static int ldb_lock_noop_callback(struct ldb_request *req,
+ struct ldb_reply *ares)
+{
+ return LDB_ERR_OPERATIONS_ERROR;
+}
+
+struct ldb_db_lock_context {
+ struct ldb_request *unlock_req;
+ 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, extended);
+ if (next_module != NULL) {
+ ret = next_module->ops->extended(next_module, lock_context->unlock_req);
+ } 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_request *lock_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;
+ }
+
+ ret = ldb_build_extended_req(&lock_req, ldb, down_req,
+ LDB_EXTENDED_READ_LOCK_DB, NULL, NULL,
+ NULL, ldb_lock_noop_callback,
+ NULL);
+ LDB_REQ_SET_LOCATION(lock_req);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ ret = ldb_build_extended_req(&lock_context->unlock_req, ldb, down_req,
+ LDB_EXTENDED_READ_UNLOCK_DB, NULL, NULL,
+ NULL, ldb_lock_noop_callback,
+ NULL);
+ LDB_REQ_SET_LOCATION(lock_req);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /* call DB lock */
+ FIRST_OP_NOERR(ldb, extended);
+ if (next_module != NULL) {
+ ret = next_module->ops->extended(next_module, lock_req);
+ } else {
+ ret = LDB_ERR_UNSUPPORTED_CRITICAL_EXTENSION;
+ }
+
+ TALLOC_FREE(lock_req);
+ 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 +1155,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/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index b6eec0d9719..9e9443b25e0 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -512,6 +512,9 @@ void ldb_handle_use_global_event_context(struct ldb_handle *handle);
* These extended operations allow a read lock to be taken during the
* overall search operation, not just for each DB read. This ensures
* no modification while the search is being processed.
+ *
+ * They must NOT be handled via an event loop, they must return
+ * or fail directly, and do not use the callback or data
*/
#define LDB_EXTENDED_READ_LOCK_DB "1.3.6.1.4.1.7165.4.4.10"
#define LDB_EXTENDED_READ_UNLOCK_DB "1.3.6.1.4.1.7165.4.4.11"
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 f0f19596c54d48215c293a6203fd386789e6a9fe 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 15/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 da0e219dfd6f8632fac212dbefd54e79eb88e0fc 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 16/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 eb2f2d18cfd00cd637d77f35913c41e739ee3e78 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 17/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 edc91112b00d552dac78299427c5b504741942ff 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 18/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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170614/d6d95109/signature-0001.sig>
More information about the samba-technical
mailing list