[SCM] Samba Shared Repository - branch master updated

Garming Sam garming at samba.org
Thu Jun 15 03:32:02 UTC 2017


The branch, master has been updated
       via  e244ba4 repl: Set GET_ALL_GROUP_MEMBERSHIP flag in the drepl server
       via  a7ef0f8 dsdb: Improve debug messages
       via  4384962 dsdb: Ensure replication of renames works in schema partition
       via  cf99f2c selftest: Pass the dcerpc binding object to self.waitForMessages in auth_log
       via  2f045e7 stream_terminate_connection: Prevent use-after-free
       via  b158f68 selftest: Add test for gss_krb5/ntlmssp -> SPNEGO
       via  995f5c0 selftest: Add pygensec tests for GSS-SPNEGO and Win2000 emulated SPNEGO
       via  33b818a selftest: Add a test for @ATTRIBUTES and @INDEXLIST generation
       via  f587db7 ldb: Rename module -> next_module for clarity
       via  10e7c74 dsdb: Correctly call ldb_module_done in dsdb_notification
       via  c581ab3 tdb: add run-fcntl-deadlock test
       via  2277301 ldb_tdb: Improve logging on unique index violation
       via  f6f73f7 ldb_tdb: Remove the idxptr DB before we re-index
       via  d8f3034 ldb_tdb: Check for memory allocation failure in ltdb_index_transaction_start()
       via  1ff09f0 dsdb: Provide proper errors when dsdb_schema_set_indices_and_attributes fails
      from  ee77759 selftest: pass the workgroup name to Samba3::provision()

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e244ba4a8f8dea571df6abb96324cb696af67450
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Wed Nov 16 14:44:40 2016 +1300

    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>
    
    Autobuild-User(master): Garming Sam <garming at samba.org>
    Autobuild-Date(master): Thu Jun 15 05:31:59 CEST 2017 on sn-devel-144

commit a7ef0f8be4bda18e3916bdbe8468bc1703060d94
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jun 14 14:13:18 2017 +1200

    dsdb: Improve debug messages
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 438496220f62dfd2375c71f93b21c09553af6961
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jun 14 13:12:32 2017 +1200

    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>

commit cf99f2c92391fb1652bbef93089d60b11f1b8229
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Jun 13 11:20:58 2017 +1200

    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>

commit 2f045e7fc147aab2a4c7f356f0ce834f47cdff42
Author: Garming Sam <garming at catalyst.net.nz>
Date:   Fri Jun 9 14:13:25 2017 +1200

    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>

commit b158f6832358be01f71d93111aa789d7941a835e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jun 12 14:27:53 2017 +1200

    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>

commit 995f5c03c53c3eb82010b34e5b94c3e1bf8a685b
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jun 12 14:12:53 2017 +1200

    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>

commit 33b818a510271411b848adde1e733c3c5035c041
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Jun 7 11:47:15 2017 +1200

    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>

commit f587db7c89ff3add793fe54fc52629f93f0d6783
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 31 10:44:34 2017 +1200

    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>

commit 10e7c749e7b7a4155669c6ecf97a9ac52b13110a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 31 12:22:28 2017 +1200

    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>

commit c581ab3ac5099b3509a15f9f6abf3ab7512dd70b
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Apr 11 17:21:20 2017 +0200

    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>

commit 2277301e46614154977b242d38669673eee5fe25
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Jun 9 14:15:19 2017 +1200

    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>

commit f6f73f70919e3360197713e378d49e6eecfe2f1c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Jun 9 14:09:30 2017 +1200

    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>

commit d8f3034c163831c5143d95fd803b8c765bf5767f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Jun 9 14:07:40 2017 +1200

    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>

commit 1ff09f0f82a97e2950b9fddc15a3cf58fef3bc23
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Jun 9 12:06:37 2017 +1200

    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>

-----------------------------------------------------------------------

Summary of changes:
 lib/ldb/common/ldb.c                               |  80 ++++----
 lib/ldb/ldb_tdb/ldb_index.c                        |  31 +++-
 lib/tdb/test/run-fcntl-deadlock.c                  | 202 +++++++++++++++++++++
 lib/tdb/wscript                                    |   1 +
 python/samba/kcc/__init__.py                       |   1 -
 python/samba/tests/auth_log.py                     |  14 +-
 python/samba/tests/auth_log_samlogon.py            |   2 +-
 python/samba/tests/dsdb_schema_attributes.py       | 138 ++++++++++++++
 python/samba/tests/gensec.py                       |  70 ++++++-
 source4/dsdb/common/util.c                         |  13 ++
 source4/dsdb/kcc/kcc_periodic.c                    |   1 -
 source4/dsdb/repl/drepl_out_helpers.c              |  14 ++
 source4/dsdb/samdb/ldb_modules/dsdb_notification.c |   2 +-
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c    |  13 +-
 source4/dsdb/samdb/ldb_modules/schema_data.c       |   8 +-
 source4/dsdb/samdb/ldb_modules/util.h              |   1 +
 source4/dsdb/schema/schema_set.c                   |   8 +
 source4/selftest/tests.py                          |   3 +
 source4/setup/schema_samba4.ldif                   |   1 +
 source4/smbd/service_stream.c                      |  10 +
 source4/torture/drs/python/repl_schema.py          |  31 ++++
 21 files changed, 580 insertions(+), 64 deletions(-)
 create mode 100644 lib/tdb/test/run-fcntl-deadlock.c
 create mode 100644 python/samba/tests/dsdb_schema_attributes.py


Changeset truncated at 500 lines:

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index a3a0002..c950330 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;
 	}
 
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 721ec1c..232bb4c 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;
 }
 
@@ -1175,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;
 	}
 
@@ -1651,6 +1668,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
 	 */
diff --git a/lib/tdb/test/run-fcntl-deadlock.c b/lib/tdb/test/run-fcntl-deadlock.c
new file mode 100644
index 0000000..0a328af
--- /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) {


-- 
Samba Shared Repository



More information about the samba-cvs mailing list