[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Wed Dec 20 19:32:02 UTC 2017


The branch, master has been updated
       via  576fb4f g_lock: fix cleanup of stale entries in g_lock_trylock()
       via  1b1f9bf torture3: add LOCAL-G-LOCK6 test
       via  23a62c9 dsdb: Improve code and directly close fp
       via  287236e dsdb: Fix CID 1426728 Structurally dead code
       via  3242bce dsdb: Fix CID 1426727 Resource leak
      from  676261f selftest: replace global with explicit environment variables

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


- Log -----------------------------------------------------------------
commit 576fb4fb5dc506bf55e5cf87973999dca444149b
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 20 08:25:19 2017 +0100

    g_lock: fix cleanup of stale entries in g_lock_trylock()
    
    g_lock_trylock() always incremented the counter 'i', even after cleaning a stale
    entry at position 'i', which means it skipped checking for a conflict against
    the new entry at position 'i'.
    
    As result a process could get a write lock, while there're still
    some read lock holders. Once we get into that problem, also more than
    one write lock are possible.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13195
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Wed Dec 20 20:31:48 CET 2017 on sn-devel-144

commit 1b1f9bfd294a1cc2c7892cd5aa7bf62f6d18dcb4
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 20 09:44:40 2017 +0100

    torture3: add LOCAL-G-LOCK6 test
    
    This is a regression test for bug #13195.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13195
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 23a62c9f831d5deec753dea9b1c60b8ac7601d14
Author: Andreas Schneider <asn at samba.org>
Date:   Tue Dec 19 15:42:14 2017 +0100

    dsdb: Improve code and directly close fp
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 287236eac9267977735935ead4e68b2dc2f3468b
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 19 14:13:37 2017 +0100

    dsdb: Fix CID 1426728 Structurally dead code
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>

commit 3242bce63630c69a9221ad397d9e07560ff17769
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Dec 19 14:11:24 2017 +0100

    dsdb: Fix CID 1426727 Resource leak
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>

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

Summary of changes:
 source3/lib/g_lock.c                               |   6 +-
 source3/selftest/tests.py                          |   1 +
 source3/torture/proto.h                            |   1 +
 source3/torture/test_g_lock.c                      | 152 +++++++++++++++++++++
 source3/torture/torture.c                          |   1 +
 source4/dsdb/samdb/ldb_modules/encrypted_secrets.c |   7 +-
 6 files changed, 162 insertions(+), 6 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 50ea566..68a9ab3 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -230,7 +230,9 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 		}
 	}
 
-	for (i=0; i<lck.num_recs; i++) {
+	i = 0;
+
+	while (i < lck.num_recs) {
 		struct g_lock_rec lock;
 
 		g_lock_get_rec(&lck, i, &lock);
@@ -269,7 +271,9 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 			 */
 			g_lock_rec_del(&lck, i);
 			modified = true;
+			continue;
 		}
+		i++;
 	}
 
 	modified = true;
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d4e919d..34de4be 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -158,6 +158,7 @@ local_tests = [
     "LOCAL-G-LOCK3",
     "LOCAL-G-LOCK4",
     "LOCAL-G-LOCK5",
+    "LOCAL-G-LOCK6",
     "LOCAL-NAMEMAP-CACHE1",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 12c76a7..7ab2fe1 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -132,6 +132,7 @@ bool run_g_lock2(int dummy);
 bool run_g_lock3(int dummy);
 bool run_g_lock4(int dummy);
 bool run_g_lock5(int dummy);
+bool run_g_lock6(int dummy);
 bool run_g_lock_ping_pong(int dummy);
 bool run_local_namemap_cache1(int dummy);
 
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index 253d20c..718c196 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -695,6 +695,158 @@ bool run_g_lock5(int dummy)
 	return true;
 }
 
+struct lock6_parser_state {
+	size_t num_locks;
+};
+
+static void lock6_parser(const struct g_lock_rec *locks,
+			 size_t num_locks,
+			 const uint8_t *data,
+			 size_t datalen,
+			 void *private_data)
+{
+	struct lock6_parser_state *state = private_data;
+	state->num_locks = num_locks;
+}
+
+/*
+ * Test cleanup with contention and stale locks
+ */
+
+bool run_g_lock6(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock6";
+	pid_t child;
+	int exit_pipe[2], ready_pipe[2];
+	NTSTATUS status;
+	size_t i, nprocs;
+	int ret;
+	bool ok;
+	ssize_t nread;
+	char c;
+
+	if ((pipe(exit_pipe) != 0) || (pipe(ready_pipe) != 0)) {
+		perror("pipe failed");
+		return false;
+	}
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		fprintf(stderr, "get_g_lock_ctx failed");
+		return false;
+	}
+
+	nprocs = 2;
+	for (i=0; i<nprocs; i++) {
+
+		child = fork();
+
+		if (child == -1) {
+			perror("fork failed");
+			return false;
+		}
+
+		if (child == 0) {
+			TALLOC_FREE(ctx);
+
+			status = reinit_after_fork(msg, ev, false, "");
+			if (!NT_STATUS_IS_OK(status)) {
+				fprintf(stderr, "reinit_after_fork failed: %s\n",
+					nt_errstr(status));
+				exit(1);
+			}
+
+			close(ready_pipe[0]);
+			close(exit_pipe[1]);
+
+			ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+			if (!ok) {
+				fprintf(stderr, "get_g_lock_ctx failed");
+				exit(1);
+			}
+			status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+					     (struct timeval) { .tv_sec = 1 });
+			if (!NT_STATUS_IS_OK(status)) {
+				fprintf(stderr,
+					"child g_lock_lock failed %s\n",
+					nt_errstr(status));
+				exit(1);
+			}
+			if (i == 0) {
+				exit(0);
+			}
+			close(ready_pipe[1]);
+			nread = sys_read(exit_pipe[0], &c, sizeof(c));
+			if (nread != 0) {
+				fprintf(stderr, "sys_read returned %zu (%s)\n",
+					nread, strerror(errno));
+				exit(1);
+			}
+			exit(0);
+		}
+	}
+
+	close(ready_pipe[1]);
+
+	nread = sys_read(ready_pipe[0], &c, sizeof(c));
+	if (nread != 0) {
+		fprintf(stderr, "sys_read returned %zd (%s)\n",
+			nread, strerror(errno));
+		return false;
+	}
+
+	{
+		int child_status;
+		ret = waitpid(-1, &child_status, 0);
+		if (ret == -1) {
+			perror("waitpid failed");
+			return false;
+		}
+	}
+
+	{
+		struct lock6_parser_state state;
+
+		status = g_lock_dump(ctx, lockname, lock6_parser, &state);
+		if (!NT_STATUS_IS_OK(status)) {
+			fprintf(stderr, "g_lock_dump returned %s\n",
+				nt_errstr(status));
+			return false;
+		}
+
+		if (state.num_locks != nprocs) {
+			fprintf(stderr, "nlocks=%zu, expected %zu\n",
+				state.num_locks, nprocs);
+			return false;
+		}
+
+		status = g_lock_lock(ctx, lockname, G_LOCK_WRITE,
+				     (struct timeval) { .tv_sec = 1 });
+		if (!NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
+			fprintf(stderr, "g_lock_lock should have failed with %s - %s\n",
+				nt_errstr(NT_STATUS_IO_TIMEOUT),
+				nt_errstr(status));
+			return false;
+		}
+	}
+
+	close(exit_pipe[1]);
+
+	{
+		int child_status;
+		ret = waitpid(-1, &child_status, 0);
+		if (ret == -1) {
+			perror("waitpid failed");
+			return false;
+		}
+	}
+
+	return true;
+}
+
 extern int torture_numops;
 extern int torture_nprocs;
 
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 232e8ff..1a9cf5b 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11585,6 +11585,7 @@ static struct {
 	{ "LOCAL-G-LOCK3", run_g_lock3, 0 },
 	{ "LOCAL-G-LOCK4", run_g_lock4, 0 },
 	{ "LOCAL-G-LOCK5", run_g_lock5, 0 },
+	{ "LOCAL-G-LOCK6", run_g_lock6, 0 },
 	{ "LOCAL-G-LOCK-PING-PONG", run_g_lock_ping_pong, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "LOCAL-NAMEMAP-CACHE1", run_local_namemap_cache1, 0 },
diff --git a/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c b/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
index bc03fee..8f3e049 100644
--- a/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
+++ b/source4/dsdb/samdb/ldb_modules/encrypted_secrets.c
@@ -312,7 +312,9 @@ static int load_keys(struct ldb_module *module, struct es_data *data)
 	}
 
 	read = fread(key.data, 1, key.length, fp);
+	fclose(fp);
 	if (read == 0) {
+		TALLOC_FREE(frame);
 		ldb_debug(ldb,
 			  LDB_DEBUG_WARNING,
 			  "Zero length encrypted secrets key file. "
@@ -323,7 +325,6 @@ static int load_keys(struct ldb_module *module, struct es_data *data)
 	}
 	if (read != key.length) {
 		TALLOC_FREE(frame);
-		fclose(fp);
 		if (errno) {
 			log_error(ldb,
 				  errno,
@@ -338,8 +339,6 @@ static int load_keys(struct ldb_module *module, struct es_data *data)
 		}
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
-	fclose(fp);
-
 
 	data->keys[0] = key;
 	data->encrypt_secrets = true;
@@ -1666,8 +1665,6 @@ static int es_modify(struct ldb_module *module, struct ldb_request *req)
 		}
 		return ldb_next_request(module, new_req);
 	}
-	req->op.add.message = encrypted_msg;
-	return ldb_next_request(module, req);
 }
 
 static int es_delete(struct ldb_module *module, struct ldb_request *req)


-- 
Samba Shared Repository



More information about the samba-cvs mailing list