[PATCH] Protect Samba against different versions in a cluster

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue May 30 13:35:13 UTC 2017


Hi!

The attached patchset extends g_lock and makes sure that only one
version of smbd runs simultaneously in a cluster. This will probably
have to be modified once we actually start supporting rolling code
upgrade and different versions in databases, but for now this is a
simple protection against accidents due to different smbds running on
the same locking.tdb.

Review appreciated!

Thanks, Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From b7fbe6e1139f2ac6acd8ae3c84132bde59090bff Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 24 May 2017 13:27:18 +0200
Subject: [PATCH 01/20] g_lock: Fix two typos

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 1815796..5b282e8 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -382,7 +382,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 		}
 	}
 	if (i == num_locks) {
-		DEBUG(10, ("g_lock_force_unlock: Lock not found\n"));
+		DBG_DEBUG("Lock not found, num_locks=%u\n", num_locks);
 		status = NT_STATUS_NOT_FOUND;
 		goto done;
 	}
@@ -399,8 +399,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 		status = dbwrap_record_store(rec, data, 0);
 	}
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("g_lock_force_unlock: Could not store record: %s\n",
-			  nt_errstr(status)));
+		DBG_WARNING("Could not store record: %s\n", nt_errstr(status));
 		goto done;
 	}
 
-- 
2.1.4


From 2ab430bf0830177100997f68182e2c30dcb03f5f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 16 May 2017 15:05:49 +0200
Subject: [PATCH 02/20] torture3: Initial test g_lock

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py     |   1 +
 source3/torture/proto.h       |   1 +
 source3/torture/test_g_lock.c | 104 ++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c     |   1 +
 source3/wscript_build         |   1 +
 5 files changed, 108 insertions(+)
 create mode 100644 source3/torture/test_g_lock.c

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 5f707c0..768aeb1 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -148,6 +148,7 @@ local_tests = [
     "LOCAL-CANONICALIZE-PATH",
     "LOCAL-DBWRAP-WATCH1",
     "LOCAL-DBWRAP-WATCH2",
+    "LOCAL-G-LOCK1",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 129c05d..f93100a 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -124,5 +124,6 @@ bool run_messaging_fdpass2a(int dummy);
 bool run_messaging_fdpass2b(int dummy);
 bool run_oplock_cancel(int dummy);
 bool run_pthreadpool_tevent(int dummy);
+bool run_g_lock1(int dummy);
 
 #endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
new file mode 100644
index 0000000..222a298
--- /dev/null
+++ b/source3/torture/test_g_lock.c
@@ -0,0 +1,104 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Test g_lock API
+ * Copyright (C) Volker Lendecke 2017
+ *
+ * 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/>.
+ */
+
+#include "includes.h"
+#include "torture/proto.h"
+#include "system/filesys.h"
+#include "g_lock.h"
+#include "messages.h"
+
+static bool get_g_lock_ctx(TALLOC_CTX *mem_ctx,
+			   struct tevent_context **ev,
+			   struct messaging_context **msg,
+			   struct g_lock_ctx **ctx)
+{
+	*ev = samba_tevent_context_init(mem_ctx);
+	if (*ev == NULL) {
+		fprintf(stderr, "tevent_context_init failed\n");
+		return false;
+	}
+	*msg = messaging_init(*ev, *ev);
+	if (*msg == NULL) {
+		fprintf(stderr, "messaging_init failed\n");
+		TALLOC_FREE(*ev);
+		return false;
+	}
+	*ctx = g_lock_ctx_init(*ev, *msg);
+	if (*ctx == NULL) {
+		fprintf(stderr, "g_lock_ctx_init failed\n");
+		TALLOC_FREE(*msg);
+		TALLOC_FREE(*ev);
+		return false;
+	}
+
+	return true;
+}
+
+bool run_g_lock1(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock1";
+	NTSTATUS status;
+	bool ret = false;
+	bool ok;
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_lock failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)) {
+		fprintf(stderr, "Double lock got %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_unlock(ctx, lockname);
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_unlock failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_unlock(ctx, lockname);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+		fprintf(stderr, "g_lock_unlock returned: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	ret = true;
+fail:
+	TALLOC_FREE(ctx);
+	TALLOC_FREE(msg);
+	TALLOC_FREE(ev);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index bdcf1e1..5ef4d4e 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11477,6 +11477,7 @@ static struct {
 	{ "LOCAL-DBWRAP-CTDB", run_local_dbwrap_ctdb, 0 },
 	{ "LOCAL-BENCH-PTHREADPOOL", run_bench_pthreadpool, 0 },
 	{ "LOCAL-PTHREADPOOL-TEVENT", run_pthreadpool_tevent, 0 },
+	{ "LOCAL-G-LOCK1", run_g_lock1, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 },
 	{NULL, NULL, 0}};
diff --git a/source3/wscript_build b/source3/wscript_build
index d5bb62a..2cc74e0 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -1212,6 +1212,7 @@ bld.SAMBA3_BINARY('smbtorture' + bld.env.suffix3,
                         torture/test_pthreadpool_tevent.c
                         torture/bench_pthreadpool.c
                         torture/wbc_async.c
+                        torture/test_g_lock.c
                         ''',
                  deps='''
                       talloc
-- 
2.1.4


From 795ebc7f92dc6be21029cb8fe7d9cdc0d3963d52 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 05:54:36 +0200
Subject: [PATCH 03/20] g_lock: More correct error msg

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 5b282e8..37cf70c 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -131,7 +131,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 	data = dbwrap_record_get_value(rec);
 
 	if (!g_lock_parse(talloc_tos(), data, &num_locks, &locks)) {
-		return NT_STATUS_INTERNAL_ERROR;
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
 	for (i=0; i<num_locks; i++) {
-- 
2.1.4


From 0fa07001207a0d595dcf418ceb684755428af902 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 05:52:56 +0200
Subject: [PATCH 04/20] g_lock: Make it endian-neutral

Add explicit parsing

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 98 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 85 insertions(+), 13 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 37cf70c..f9eb5ef 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -45,6 +45,22 @@ struct g_lock_rec {
 	struct server_id pid;
 };
 
+#define G_LOCK_REC_LENGTH (SERVER_ID_BUF_LENGTH+1)
+
+static void g_lock_rec_put(uint8_t buf[G_LOCK_REC_LENGTH],
+			   const struct g_lock_rec rec)
+{
+	SCVAL(buf, 0, rec.lock_type);
+	server_id_put(buf+1, rec.pid);
+}
+
+static void g_lock_rec_get(struct g_lock_rec *rec,
+			   const uint8_t buf[G_LOCK_REC_LENGTH])
+{
+	rec->lock_type = CVAL(buf, 0);
+	server_id_get(&rec->pid, buf+1);
+}
+
 struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
 				   struct messaging_context *msg)
 {
@@ -100,24 +116,85 @@ static bool g_lock_conflicts(enum g_lock_type l1, enum g_lock_type l2)
 static bool g_lock_parse(TALLOC_CTX *mem_ctx, TDB_DATA data,
 			 unsigned *pnum_locks, struct g_lock_rec **plocks)
 {
-	unsigned num_locks;
+	size_t i, num_locks;
 	struct g_lock_rec *locks;
 
-	if ((data.dsize % sizeof(struct g_lock_rec)) != 0) {
+	if ((data.dsize % G_LOCK_REC_LENGTH) != 0) {
 		DEBUG(1, ("invalid lock record length %zu\n", data.dsize));
 		return false;
 	}
-	num_locks = data.dsize / sizeof(struct g_lock_rec);
-	locks = talloc_memdup(mem_ctx, data.dptr, data.dsize);
+	num_locks = data.dsize / G_LOCK_REC_LENGTH;
+
+	locks = talloc_array(mem_ctx, struct g_lock_rec, num_locks);
 	if (locks == NULL) {
 		DEBUG(1, ("talloc_memdup failed\n"));
 		return false;
 	}
+
+	for (i=0; i<num_locks; i++) {
+		g_lock_rec_get(&locks[i], data.dptr);
+		data.dptr += G_LOCK_REC_LENGTH;
+	}
+
 	*plocks = locks;
 	*pnum_locks = num_locks;
 	return true;
 }
 
+static ssize_t g_lock_unparse(uint8_t *buf, size_t buflen,
+			      const struct g_lock_rec *locks,
+			      size_t num_locks)
+{
+	size_t i, len, ofs;
+
+	if (num_locks > UINT32_MAX/G_LOCK_REC_LENGTH) {
+		return -1;
+	}
+
+	len = num_locks * G_LOCK_REC_LENGTH;
+
+	if (len > buflen) {
+		return len;
+	}
+
+	ofs = 0;
+
+	for (i=0; i<num_locks; i++) {
+		g_lock_rec_put(buf+ofs, locks[i]);
+		ofs += G_LOCK_REC_LENGTH;
+	}
+
+	return len;
+}
+
+static NTSTATUS g_lock_record_store(struct db_record *rec,
+				    const struct g_lock_rec *locks,
+				    size_t num_locks)
+{
+	ssize_t len;
+	uint8_t *buf;
+	NTSTATUS status;
+
+	len = g_lock_unparse(NULL, 0, locks, num_locks);
+	if (len == -1) {
+		return NT_STATUS_BUFFER_TOO_SMALL;
+	}
+
+	buf = talloc_array(rec, uint8_t, len);
+	if (buf == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	g_lock_unparse(buf, len, locks, num_locks);
+
+	status = dbwrap_record_store(
+		rec, (TDB_DATA) { .dptr = buf, .dsize = len }, 0);
+
+	TALLOC_FREE(buf);
+
+	return status;
+}
+
 static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 			       enum g_lock_type type,
 			       struct server_id *blocker)
@@ -182,12 +259,10 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 done:
 	if (modified) {
 		NTSTATUS store_status;
-
-		data = make_tdb_data((uint8_t *)locks, num_locks * sizeof(*locks));
-		store_status = dbwrap_record_store(rec, data, 0);
+		store_status = g_lock_record_store(rec, locks, num_locks);
 		if (!NT_STATUS_IS_OK(store_status)) {
-			DEBUG(1, ("rec->store failed: %s\n",
-				  nt_errstr(store_status)));
+			DBG_WARNING("g_lock_record_store failed: %s\n",
+				    nt_errstr(store_status));
 			status = store_status;
 		}
 	}
@@ -393,10 +468,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 	if (num_locks == 0) {
 		status = dbwrap_record_delete(rec);
 	} else {
-		TDB_DATA data;
-		data = make_tdb_data((uint8_t *)locks,
-				     sizeof(struct g_lock_rec) * num_locks);
-		status = dbwrap_record_store(rec, data, 0);
+		status = g_lock_record_store(rec, locks, num_locks);
 	}
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_WARNING("Could not store record: %s\n", nt_errstr(status));
-- 
2.1.4


From ee0214e1bf86bf4181d81e998a5aa1610e4dcf1a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 16:40:45 +0200
Subject: [PATCH 05/20] g_lock: Remove unused g_lock_get

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/g_lock.h |  2 --
 source3/lib/g_lock.c     | 33 ---------------------------------
 2 files changed, 35 deletions(-)

diff --git a/source3/include/g_lock.h b/source3/include/g_lock.h
index f513349..998a9da85 100644
--- a/source3/include/g_lock.h
+++ b/source3/include/g_lock.h
@@ -42,8 +42,6 @@ NTSTATUS g_lock_lock_recv(struct tevent_req *req);
 NTSTATUS g_lock_lock(struct g_lock_ctx *ctx, const char *name,
 		     enum g_lock_type lock_type, struct timeval timeout);
 NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name);
-NTSTATUS g_lock_get(struct g_lock_ctx *ctx, const char *name,
-		struct server_id *pid);
 
 NTSTATUS g_lock_do(const char *name, enum g_lock_type lock_type,
 		   struct timeval timeout,
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index f9eb5ef..d0302cd 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -558,39 +558,6 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
 	return NT_STATUS_OK;
 }
 
-struct g_lock_get_state {
-	bool found;
-	struct server_id *pid;
-};
-
-static int g_lock_get_fn(struct server_id pid, enum g_lock_type lock_type,
-			 void *priv)
-{
-	struct g_lock_get_state *state = (struct g_lock_get_state *)priv;
-	state->found = true;
-	*state->pid = pid;
-	return 1;
-}
-
-NTSTATUS g_lock_get(struct g_lock_ctx *ctx, const char *name,
-		    struct server_id *pid)
-{
-	struct g_lock_get_state state;
-	NTSTATUS status;
-
-	state.found = false;
-	state.pid = pid;
-
-	status = g_lock_dump(ctx, name, g_lock_get_fn, &state);
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-	if (!state.found) {
-		return NT_STATUS_NOT_FOUND;
-	}
-	return NT_STATUS_OK;
-}
-
 static bool g_lock_init_all(TALLOC_CTX *mem_ctx,
 			    struct tevent_context **pev,
 			    struct messaging_context **pmsg,
-- 
2.1.4


From f68b73bad0dd77b617cd4eb1d1b2a1553b66122e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 16:43:01 +0200
Subject: [PATCH 06/20] g_lock: Remove a pointless "else"

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index d0302cd..62495ae 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -514,9 +514,8 @@ int g_lock_locks(struct g_lock_ctx *ctx,
 	status = dbwrap_traverse_read(ctx->db, g_lock_locks_fn, &state, &count);
 	if (!NT_STATUS_IS_OK(status)) {
 		return -1;
-	} else {
-		return count;
 	}
+	return count;
 }
 
 NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
-- 
2.1.4


From 412a7698b5b216a539191a391dc937a84ea23d57 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 16:53:14 +0200
Subject: [PATCH 07/20] g_lock: parse->get

Make it more in line with server_id_get/put

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 62495ae..becc047 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -113,8 +113,8 @@ static bool g_lock_conflicts(enum g_lock_type l1, enum g_lock_type l2)
 	return true;
 }
 
-static bool g_lock_parse(TALLOC_CTX *mem_ctx, TDB_DATA data,
-			 unsigned *pnum_locks, struct g_lock_rec **plocks)
+static bool g_lock_get(TALLOC_CTX *mem_ctx, TDB_DATA data,
+		       unsigned *pnum_locks, struct g_lock_rec **plocks)
 {
 	size_t i, num_locks;
 	struct g_lock_rec *locks;
@@ -207,7 +207,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 
 	data = dbwrap_record_get_value(rec);
 
-	if (!g_lock_parse(talloc_tos(), data, &num_locks, &locks)) {
+	if (!g_lock_get(talloc_tos(), data, &num_locks, &locks)) {
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
@@ -446,8 +446,8 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 
 	value = dbwrap_record_get_value(rec);
 
-	if (!g_lock_parse(talloc_tos(), value, &num_locks, &locks)) {
-		DEBUG(10, ("g_lock_parse for %s failed\n", name));
+	if (!g_lock_get(talloc_tos(), value, &num_locks, &locks)) {
+		DEBUG(10, ("g_lock_get for %s failed\n", name));
 		status = NT_STATUS_FILE_INVALID;
 		goto done;
 	}
@@ -539,12 +539,12 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
 		return NT_STATUS_OK;
 	}
 
-	ret = g_lock_parse(talloc_tos(), data, &num_locks, &locks);
+	ret = g_lock_get(talloc_tos(), data, &num_locks, &locks);
 
 	TALLOC_FREE(data.dptr);
 
 	if (!ret) {
-		DEBUG(10, ("g_lock_parse for %s failed\n", name));
+		DEBUG(10, ("g_lock_get for %s failed\n", name));
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-- 
2.1.4


From bb83a9c6e115050a0cbcbe757df1016201007c23 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 May 2017 16:53:14 +0200
Subject: [PATCH 08/20] g_lock: unparse->put

Make it more in line with server_id_get/put

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index becc047..d0d835c 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -141,9 +141,9 @@ static bool g_lock_get(TALLOC_CTX *mem_ctx, TDB_DATA data,
 	return true;
 }
 
-static ssize_t g_lock_unparse(uint8_t *buf, size_t buflen,
-			      const struct g_lock_rec *locks,
-			      size_t num_locks)
+static ssize_t g_lock_put(uint8_t *buf, size_t buflen,
+			  const struct g_lock_rec *locks,
+			  size_t num_locks)
 {
 	size_t i, len, ofs;
 
@@ -175,7 +175,7 @@ static NTSTATUS g_lock_record_store(struct db_record *rec,
 	uint8_t *buf;
 	NTSTATUS status;
 
-	len = g_lock_unparse(NULL, 0, locks, num_locks);
+	len = g_lock_put(NULL, 0, locks, num_locks);
 	if (len == -1) {
 		return NT_STATUS_BUFFER_TOO_SMALL;
 	}
@@ -185,7 +185,7 @@ static NTSTATUS g_lock_record_store(struct db_record *rec,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	g_lock_unparse(buf, len, locks, num_locks);
+	g_lock_put(buf, len, locks, num_locks);
 
 	status = dbwrap_record_store(
 		rec, (TDB_DATA) { .dptr = buf, .dsize = len }, 0);
-- 
2.1.4


From a84cb91ccb0e1c552de258f78b5464a9808ef178 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 May 2017 10:37:30 +0200
Subject: [PATCH 09/20] g_lock: Move parsing routines together

No code change, just shuffling around:

Before this patchset, g_lock_parse was somewhere in the middle. This carries no
real logic, put it on top.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 108 +++++++++++++++++++++++++--------------------------
 1 file changed, 54 insertions(+), 54 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index d0d835c..542abe4 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -61,6 +61,60 @@ static void g_lock_rec_get(struct g_lock_rec *rec,
 	server_id_get(&rec->pid, buf+1);
 }
 
+static ssize_t g_lock_put(uint8_t *buf, size_t buflen,
+			  const struct g_lock_rec *locks,
+			  size_t num_locks)
+{
+	size_t i, len, ofs;
+
+	if (num_locks > UINT32_MAX/G_LOCK_REC_LENGTH) {
+		return -1;
+	}
+
+	len = num_locks * G_LOCK_REC_LENGTH;
+
+	if (len > buflen) {
+		return len;
+	}
+
+	ofs = 0;
+
+	for (i=0; i<num_locks; i++) {
+		g_lock_rec_put(buf+ofs, locks[i]);
+		ofs += G_LOCK_REC_LENGTH;
+	}
+
+	return len;
+}
+
+static bool g_lock_get(TALLOC_CTX *mem_ctx, TDB_DATA data,
+		       unsigned *pnum_locks, struct g_lock_rec **plocks)
+{
+	size_t i, num_locks;
+	struct g_lock_rec *locks;
+
+	if ((data.dsize % G_LOCK_REC_LENGTH) != 0) {
+		DEBUG(1, ("invalid lock record length %zu\n", data.dsize));
+		return false;
+	}
+	num_locks = data.dsize / G_LOCK_REC_LENGTH;
+
+	locks = talloc_array(mem_ctx, struct g_lock_rec, num_locks);
+	if (locks == NULL) {
+		DEBUG(1, ("talloc_memdup failed\n"));
+		return false;
+	}
+
+	for (i=0; i<num_locks; i++) {
+		g_lock_rec_get(&locks[i], data.dptr);
+		data.dptr += G_LOCK_REC_LENGTH;
+	}
+
+	*plocks = locks;
+	*pnum_locks = num_locks;
+	return true;
+}
+
 struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
 				   struct messaging_context *msg)
 {
@@ -113,60 +167,6 @@ static bool g_lock_conflicts(enum g_lock_type l1, enum g_lock_type l2)
 	return true;
 }
 
-static bool g_lock_get(TALLOC_CTX *mem_ctx, TDB_DATA data,
-		       unsigned *pnum_locks, struct g_lock_rec **plocks)
-{
-	size_t i, num_locks;
-	struct g_lock_rec *locks;
-
-	if ((data.dsize % G_LOCK_REC_LENGTH) != 0) {
-		DEBUG(1, ("invalid lock record length %zu\n", data.dsize));
-		return false;
-	}
-	num_locks = data.dsize / G_LOCK_REC_LENGTH;
-
-	locks = talloc_array(mem_ctx, struct g_lock_rec, num_locks);
-	if (locks == NULL) {
-		DEBUG(1, ("talloc_memdup failed\n"));
-		return false;
-	}
-
-	for (i=0; i<num_locks; i++) {
-		g_lock_rec_get(&locks[i], data.dptr);
-		data.dptr += G_LOCK_REC_LENGTH;
-	}
-
-	*plocks = locks;
-	*pnum_locks = num_locks;
-	return true;
-}
-
-static ssize_t g_lock_put(uint8_t *buf, size_t buflen,
-			  const struct g_lock_rec *locks,
-			  size_t num_locks)
-{
-	size_t i, len, ofs;
-
-	if (num_locks > UINT32_MAX/G_LOCK_REC_LENGTH) {
-		return -1;
-	}
-
-	len = num_locks * G_LOCK_REC_LENGTH;
-
-	if (len > buflen) {
-		return len;
-	}
-
-	ofs = 0;
-
-	for (i=0; i<num_locks; i++) {
-		g_lock_rec_put(buf+ofs, locks[i]);
-		ofs += G_LOCK_REC_LENGTH;
-	}
-
-	return len;
-}
-
 static NTSTATUS g_lock_record_store(struct db_record *rec,
 				    const struct g_lock_rec *locks,
 				    size_t num_locks)
-- 
2.1.4


From bf885c735096a965d2e54ebcee5f36e65fd518b5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 May 2017 13:59:20 +0200
Subject: [PATCH 10/20] g_lock: Reformat to allow userdata

The next patches will make g_locks carry data. This
prepares the on-disk format.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 129 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 98 insertions(+), 31 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 542abe4..ef950d6 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -63,7 +63,8 @@ static void g_lock_rec_get(struct g_lock_rec *rec,
 
 static ssize_t g_lock_put(uint8_t *buf, size_t buflen,
 			  const struct g_lock_rec *locks,
-			  size_t num_locks)
+			  size_t num_locks,
+			  const uint8_t *data, size_t datalen)
 {
 	size_t i, len, ofs;
 
@@ -73,46 +74,106 @@ static ssize_t g_lock_put(uint8_t *buf, size_t buflen,
 
 	len = num_locks * G_LOCK_REC_LENGTH;
 
+	len += sizeof(uint32_t);
+	if (len < sizeof(uint32_t)) {
+		return -1;
+	}
+
+	len += datalen;
+	if (len < datalen) {
+		return -1;
+	}
+
 	if (len > buflen) {
 		return len;
 	}
 
 	ofs = 0;
+	SIVAL(buf, ofs, num_locks);
+	ofs += sizeof(uint32_t);
 
 	for (i=0; i<num_locks; i++) {
 		g_lock_rec_put(buf+ofs, locks[i]);
 		ofs += G_LOCK_REC_LENGTH;
 	}
 
+	if ((data != NULL) && (datalen != 0)) {
+		memcpy(buf+ofs, data, datalen);
+	}
+
 	return len;
 }
 
-static bool g_lock_get(TALLOC_CTX *mem_ctx, TDB_DATA data,
-		       unsigned *pnum_locks, struct g_lock_rec **plocks)
+static ssize_t g_lock_get(TDB_DATA recval,
+			  struct g_lock_rec *locks, size_t num_locks,
+			  uint8_t **data, size_t *datalen)
 {
-	size_t i, num_locks;
-	struct g_lock_rec *locks;
+	size_t found_locks;
 
-	if ((data.dsize % G_LOCK_REC_LENGTH) != 0) {
-		DEBUG(1, ("invalid lock record length %zu\n", data.dsize));
-		return false;
+	if (recval.dsize < sizeof(uint32_t)) {
+		/* Fresh or invalid record */
+		found_locks = 0;
+		goto done;
 	}
-	num_locks = data.dsize / G_LOCK_REC_LENGTH;
 
-	locks = talloc_array(mem_ctx, struct g_lock_rec, num_locks);
-	if (locks == NULL) {
-		DEBUG(1, ("talloc_memdup failed\n"));
-		return false;
+	found_locks = IVAL(recval.dptr, 0);
+	recval.dptr += sizeof(uint32_t);
+	recval.dsize -= sizeof(uint32_t);
+
+	if (found_locks > recval.dsize/G_LOCK_REC_LENGTH) {
+		/* Invalid record */
+		return 0;
 	}
 
-	for (i=0; i<num_locks; i++) {
-		g_lock_rec_get(&locks[i], data.dptr);
-		data.dptr += G_LOCK_REC_LENGTH;
+	if (found_locks <= num_locks) {
+		size_t i;
+
+		for (i=0; i<found_locks; i++) {
+			g_lock_rec_get(&locks[i], recval.dptr);
+			recval.dptr += G_LOCK_REC_LENGTH;
+			recval.dsize -= G_LOCK_REC_LENGTH;
+		}
+	} else {
+		/*
+		 * Not enough space passed in by the caller, don't
+		 * parse the locks.
+		 */
+		recval.dptr += found_locks * G_LOCK_REC_LENGTH;
+		recval.dsize -= found_locks * G_LOCK_REC_LENGTH;
 	}
 
+done:
+	if (data != NULL) {
+		*data = recval.dptr;
+	}
+	if (datalen != NULL) {
+		*datalen = recval.dsize;
+	}
+	return found_locks;
+}
+
+static NTSTATUS g_lock_get_talloc(TALLOC_CTX *mem_ctx, TDB_DATA recval,
+				  struct g_lock_rec **plocks,
+				  size_t *pnum_locks,
+				  uint8_t **data, size_t *datalen)
+{
+	struct g_lock_rec *locks;
+	ssize_t num_locks;
+
+	num_locks = g_lock_get(recval, NULL, 0, NULL, NULL);
+	if (num_locks == -1) {
+		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	}
+	locks = talloc_array(mem_ctx, struct g_lock_rec, num_locks);
+	if (locks == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	g_lock_get(recval, locks, num_locks, data, datalen);
+
 	*plocks = locks;
 	*pnum_locks = num_locks;
-	return true;
+
+	return NT_STATUS_OK;
 }
 
 struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
@@ -175,7 +236,7 @@ static NTSTATUS g_lock_record_store(struct db_record *rec,
 	uint8_t *buf;
 	NTSTATUS status;
 
-	len = g_lock_put(NULL, 0, locks, num_locks);
+	len = g_lock_put(NULL, 0, locks, num_locks, NULL, 0);
 	if (len == -1) {
 		return NT_STATUS_BUFFER_TOO_SMALL;
 	}
@@ -185,7 +246,7 @@ static NTSTATUS g_lock_record_store(struct db_record *rec,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	g_lock_put(buf, len, locks, num_locks);
+	g_lock_put(buf, len, locks, num_locks, NULL, 0);
 
 	status = dbwrap_record_store(
 		rec, (TDB_DATA) { .dptr = buf, .dsize = len }, 0);
@@ -200,15 +261,17 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 			       struct server_id *blocker)
 {
 	TDB_DATA data;
-	unsigned i, num_locks;
+	size_t i, num_locks;
 	struct g_lock_rec *locks, *tmp;
 	NTSTATUS status;
 	bool modified = false;
 
 	data = dbwrap_record_get_value(rec);
 
-	if (!g_lock_get(talloc_tos(), data, &num_locks, &locks)) {
-		return NT_STATUS_INTERNAL_DB_CORRUPTION;
+	status = g_lock_get_talloc(talloc_tos(), data, &locks, &num_locks,
+				   NULL, NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
 	for (i=0; i<num_locks; i++) {
@@ -432,7 +495,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 	struct server_id self = messaging_server_id(ctx->msg);
 	struct db_record *rec = NULL;
 	struct g_lock_rec *locks = NULL;
-	unsigned i, num_locks;
+	size_t i, num_locks;
 	NTSTATUS status;
 	TDB_DATA value;
 
@@ -446,8 +509,11 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 
 	value = dbwrap_record_get_value(rec);
 
-	if (!g_lock_get(talloc_tos(), value, &num_locks, &locks)) {
-		DEBUG(10, ("g_lock_get for %s failed\n", name));
+	status = g_lock_get_talloc(talloc_tos(), value, &locks, &num_locks,
+				   NULL, NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("g_lock_get for %s failed: %s\n", name,
+			  nt_errstr(status));
 		status = NT_STATUS_FILE_INVALID;
 		goto done;
 	}
@@ -457,7 +523,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 		}
 	}
 	if (i == num_locks) {
-		DBG_DEBUG("Lock not found, num_locks=%u\n", num_locks);
+		DBG_DEBUG("Lock not found, num_locks=%zu\n", num_locks);
 		status = NT_STATUS_NOT_FOUND;
 		goto done;
 	}
@@ -525,9 +591,8 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
 		     void *private_data)
 {
 	TDB_DATA data;
-	unsigned i, num_locks;
+	size_t i, num_locks;
 	struct g_lock_rec *locks = NULL;
-	bool ret;
 	NTSTATUS status;
 
 	status = dbwrap_fetch_bystring(ctx->db, talloc_tos(), name, &data);
@@ -539,12 +604,14 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
 		return NT_STATUS_OK;
 	}
 
-	ret = g_lock_get(talloc_tos(), data, &num_locks, &locks);
+	status = g_lock_get_talloc(talloc_tos(), data, &locks, &num_locks,
+				   NULL, NULL);
 
 	TALLOC_FREE(data.dptr);
 
-	if (!ret) {
-		DEBUG(10, ("g_lock_get for %s failed\n", name));
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("g_lock_get for %s failed: %s\n", name,
+			  nt_errstr(status));
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-- 
2.1.4


From 328c069a3c7c56291082aa0c93aef8f7513eec11 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 May 2017 16:22:15 +0200
Subject: [PATCH 11/20] g_lock: Make g_lock_record_store also store userdata

Sequel to the previous commit changing the get/put routines for
the on-disk format

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index ef950d6..4a9d229 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -230,13 +230,14 @@ static bool g_lock_conflicts(enum g_lock_type l1, enum g_lock_type l2)
 
 static NTSTATUS g_lock_record_store(struct db_record *rec,
 				    const struct g_lock_rec *locks,
-				    size_t num_locks)
+				    size_t num_locks,
+				    const uint8_t *data, size_t datalen)
 {
 	ssize_t len;
 	uint8_t *buf;
 	NTSTATUS status;
 
-	len = g_lock_put(NULL, 0, locks, num_locks, NULL, 0);
+	len = g_lock_put(NULL, 0, locks, num_locks, data, datalen);
 	if (len == -1) {
 		return NT_STATUS_BUFFER_TOO_SMALL;
 	}
@@ -246,7 +247,7 @@ static NTSTATUS g_lock_record_store(struct db_record *rec,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	g_lock_put(buf, len, locks, num_locks, NULL, 0);
+	g_lock_put(buf, len, locks, num_locks, data, datalen);
 
 	status = dbwrap_record_store(
 		rec, (TDB_DATA) { .dptr = buf, .dsize = len }, 0);
@@ -260,7 +261,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 			       enum g_lock_type type,
 			       struct server_id *blocker)
 {
-	TDB_DATA data;
+	TDB_DATA data, userdata;
 	size_t i, num_locks;
 	struct g_lock_rec *locks, *tmp;
 	NTSTATUS status;
@@ -269,7 +270,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 	data = dbwrap_record_get_value(rec);
 
 	status = g_lock_get_talloc(talloc_tos(), data, &locks, &num_locks,
-				   NULL, NULL);
+				   &userdata.dptr, &userdata.dsize);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
@@ -322,7 +323,8 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 done:
 	if (modified) {
 		NTSTATUS store_status;
-		store_status = g_lock_record_store(rec, locks, num_locks);
+		store_status = g_lock_record_store(
+			rec, locks, num_locks, userdata.dptr, userdata.dsize);
 		if (!NT_STATUS_IS_OK(store_status)) {
 			DBG_WARNING("g_lock_record_store failed: %s\n",
 				    nt_errstr(store_status));
@@ -497,7 +499,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 	struct g_lock_rec *locks = NULL;
 	size_t i, num_locks;
 	NTSTATUS status;
-	TDB_DATA value;
+	TDB_DATA value, userdata;
 
 	rec = dbwrap_fetch_locked(ctx->db, talloc_tos(),
 				  string_term_tdb_data(name));
@@ -510,7 +512,7 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 	value = dbwrap_record_get_value(rec);
 
 	status = g_lock_get_talloc(talloc_tos(), value, &locks, &num_locks,
-				   NULL, NULL);
+				   &userdata.dptr, &userdata.dsize);
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_DEBUG("g_lock_get for %s failed: %s\n", name,
 			  nt_errstr(status));
@@ -531,10 +533,11 @@ NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name)
 	locks[i] = locks[num_locks-1];
 	num_locks -= 1;
 
-	if (num_locks == 0) {
+	if ((num_locks == 0) && (userdata.dsize == 0)) {
 		status = dbwrap_record_delete(rec);
 	} else {
-		status = g_lock_record_store(rec, locks, num_locks);
+		status = g_lock_record_store(
+			rec, locks, num_locks, userdata.dptr, userdata.dsize);
 	}
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_WARNING("Could not store record: %s\n", nt_errstr(status));
-- 
2.1.4


From 1258baef6c92d42bfba3a1209ec2627887a3ed24 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 23 May 2017 12:32:24 +0200
Subject: [PATCH 12/20] g_lock: Add g_lock_write_data

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/g_lock.h |  3 +++
 source3/lib/g_lock.c     | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/source3/include/g_lock.h b/source3/include/g_lock.h
index 998a9da85..f79e0ce 100644
--- a/source3/include/g_lock.h
+++ b/source3/include/g_lock.h
@@ -43,6 +43,9 @@ NTSTATUS g_lock_lock(struct g_lock_ctx *ctx, const char *name,
 		     enum g_lock_type lock_type, struct timeval timeout);
 NTSTATUS g_lock_unlock(struct g_lock_ctx *ctx, const char *name);
 
+NTSTATUS g_lock_write_data(struct g_lock_ctx *ctx, const char *name,
+			   const uint8_t *buf, size_t buflen);
+
 NTSTATUS g_lock_do(const char *name, enum g_lock_type lock_type,
 		   struct timeval timeout,
 		   void (*fn)(void *private_data), void *private_data);
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 4a9d229..f6c35bb 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -551,6 +551,55 @@ done:
 	return status;
 }
 
+NTSTATUS g_lock_write_data(struct g_lock_ctx *ctx, const char *name,
+			   const uint8_t *buf, size_t buflen)
+{
+	struct server_id self = messaging_server_id(ctx->msg);
+	struct db_record *rec = NULL;
+	struct g_lock_rec *locks = NULL;
+	size_t i, num_locks;
+	NTSTATUS status;
+	TDB_DATA value;
+
+	rec = dbwrap_fetch_locked(ctx->db, talloc_tos(),
+				  string_term_tdb_data(name));
+	if (rec == NULL) {
+		DEBUG(10, ("fetch_locked(\"%s\") failed\n", name));
+		status = NT_STATUS_INTERNAL_ERROR;
+		goto done;
+	}
+
+	value = dbwrap_record_get_value(rec);
+
+	status = g_lock_get_talloc(talloc_tos(), value, &locks, &num_locks,
+				   NULL, NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("g_lock_get for %s failed: %s\n", name,
+			  nt_errstr(status));
+		status = NT_STATUS_FILE_INVALID;
+		goto done;
+	}
+
+	for (i=0; i<num_locks; i++) {
+		if (server_id_equal(&self, &locks[i].pid) &&
+		    (locks[i].lock_type == G_LOCK_WRITE)) {
+			break;
+		}
+	}
+	if (i == num_locks) {
+		DBG_DEBUG("Not locked by us\n");
+		status = NT_STATUS_NOT_LOCKED;
+		goto done;
+	}
+
+	status = g_lock_record_store(rec, locks, num_locks, buf, buflen);
+
+done:
+	TALLOC_FREE(locks);
+	TALLOC_FREE(rec);
+	return status;
+}
+
 struct g_lock_locks_state {
 	int (*fn)(const char *name, void *private_data);
 	void *private_data;
-- 
2.1.4


From c42963c2a8051881f030dcb8476edf539a107116 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 May 2017 15:27:46 +0200
Subject: [PATCH 13/20] g_lock: Make g_lock_dump return a complete list of
 locks

To be honest, it did not really make sense to just pass in
lock holders individually. You could argue that it made sense
with in reality only G_LOCK_WRITE around, but soon we will have
G_LOCK_READ and thus multiple lock holders on a single lock.

Now that we also have userdata, change the g_lock_dump API

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/g_lock.h   | 13 ++++++++++---
 source3/lib/g_lock.c       | 30 +++++++++++++-----------------
 source3/utils/net_g_lock.c | 19 +++++++++++++------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/source3/include/g_lock.h b/source3/include/g_lock.h
index f79e0ce..e6d4de1 100644
--- a/source3/include/g_lock.h
+++ b/source3/include/g_lock.h
@@ -30,6 +30,11 @@ enum g_lock_type {
 	G_LOCK_WRITE = 1,
 };
 
+struct g_lock_rec {
+	enum g_lock_type lock_type;
+	struct server_id pid;
+};
+
 struct g_lock_ctx *g_lock_ctx_init(TALLOC_CTX *mem_ctx,
 				   struct messaging_context *msg);
 
@@ -54,9 +59,11 @@ int g_lock_locks(struct g_lock_ctx *ctx,
 		 int (*fn)(const char *name, void *private_data),
 		 void *private_data);
 NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
-		     int (*fn)(struct server_id pid,
-			       enum g_lock_type lock_type,
-			       void *private_data),
+		     void (*fn)(const struct g_lock_rec *locks,
+				size_t num_locks,
+				const uint8_t *data,
+				size_t datalen,
+				void *private_data),
 		     void *private_data);
 
 #endif
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index f6c35bb..db2f62a 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -40,11 +40,6 @@ struct g_lock_ctx {
  * structures.
  */
 
-struct g_lock_rec {
-	enum g_lock_type lock_type;
-	struct server_id pid;
-};
-
 #define G_LOCK_REC_LENGTH (SERVER_ID_BUF_LENGTH+1)
 
 static void g_lock_rec_put(uint8_t buf[G_LOCK_REC_LENGTH],
@@ -637,14 +632,18 @@ int g_lock_locks(struct g_lock_ctx *ctx,
 }
 
 NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
-		     int (*fn)(struct server_id pid,
-			       enum g_lock_type lock_type,
-			       void *private_data),
+		     void (*fn)(const struct g_lock_rec *locks,
+				size_t num_locks,
+				const uint8_t *data,
+				size_t datalen,
+				void *private_data),
 		     void *private_data)
 {
 	TDB_DATA data;
-	size_t i, num_locks;
+	size_t num_locks;
 	struct g_lock_rec *locks = NULL;
+	uint8_t *userdata;
+	size_t userdatalen;
 	NTSTATUS status;
 
 	status = dbwrap_fetch_bystring(ctx->db, talloc_tos(), name, &data);
@@ -657,22 +656,19 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, const char *name,
 	}
 
 	status = g_lock_get_talloc(talloc_tos(), data, &locks, &num_locks,
-				   NULL, NULL);
-
-	TALLOC_FREE(data.dptr);
+				   &userdata, &userdatalen);
 
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_DEBUG("g_lock_get for %s failed: %s\n", name,
 			  nt_errstr(status));
+		TALLOC_FREE(data.dptr);
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	for (i=0; i<num_locks; i++) {
-		if (fn(locks[i].pid, locks[i].lock_type, private_data) != 0) {
-			break;
-		}
-	}
+	fn(locks, num_locks, userdata, userdatalen, private_data);
+
 	TALLOC_FREE(locks);
+	TALLOC_FREE(data.dptr);
 	return NT_STATUS_OK;
 }
 
diff --git a/source3/utils/net_g_lock.c b/source3/utils/net_g_lock.c
index d786f5a..7543fd8 100644
--- a/source3/utils/net_g_lock.c
+++ b/source3/utils/net_g_lock.c
@@ -109,13 +109,20 @@ done:
 	return state.result;
 }
 
-static int net_g_lock_dump_fn(struct server_id pid, enum g_lock_type lock_type,
-			      void *private_data)
+static void net_g_lock_dump_fn(const struct g_lock_rec *locks,
+			       size_t num_locks,
+			       const uint8_t *data,
+			       size_t datalen,
+			       void *private_data)
 {
-	struct server_id_buf idbuf;
-	d_printf("%s: %s\n", server_id_str_buf(pid, &idbuf),
-		 (lock_type & 1) ? "WRITE" : "READ");
-	return 0;
+	size_t i;
+
+	for (i=0; i<num_locks; i++) {
+		const struct g_lock_rec *l = &locks[i];
+		struct server_id_buf idbuf;
+		d_printf("%s: %s\n", server_id_str_buf(l->pid, &idbuf),
+			 (l->lock_type & 1) ? "WRITE" : "READ");
+	}
 }
 
 static int net_g_lock_dump(struct net_context *c, int argc, const char **argv)
-- 
2.1.4


From 685b9238b336be2481cf88bee8c1cc6563496f3d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 19 May 2017 16:59:06 +0200
Subject: [PATCH 14/20] torture3: Test g_lock_write_data

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py     |  1 +
 source3/torture/proto.h       |  1 +
 source3/torture/test_g_lock.c | 96 +++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c     |  1 +
 4 files changed, 99 insertions(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 768aeb1..7ad75b7 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -149,6 +149,7 @@ local_tests = [
     "LOCAL-DBWRAP-WATCH1",
     "LOCAL-DBWRAP-WATCH2",
     "LOCAL-G-LOCK1",
+    "LOCAL-G-LOCK2",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index f93100a..df2a241 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -125,5 +125,6 @@ bool run_messaging_fdpass2b(int dummy);
 bool run_oplock_cancel(int dummy);
 bool run_pthreadpool_tevent(int dummy);
 bool run_g_lock1(int dummy);
+bool run_g_lock2(int dummy);
 
 #endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index 222a298..5a1ee60 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -102,3 +102,99 @@ fail:
 	TALLOC_FREE(ev);
 	return ret;
 }
+
+struct lock2_parser_state {
+	uint8_t *rdata;
+	bool ok;
+};
+
+static void lock2_parser(const struct g_lock_rec *locks,
+			 size_t num_locks,
+			 const uint8_t *data,
+			 size_t datalen,
+			 void *private_data)
+{
+	struct lock2_parser_state *state = private_data;
+
+	if (datalen != sizeof(uint8_t)) {
+		return;
+	}
+	*state->rdata = *data;
+	state->ok = true;
+}
+
+/*
+ * Test g_lock_write_data
+ */
+
+bool run_g_lock2(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock2";
+	uint8_t data = 42;
+	uint8_t rdata;
+	struct lock2_parser_state state = { .rdata = &rdata };
+	NTSTATUS status;
+	bool ret = false;
+	bool ok;
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		goto fail;
+	}
+
+	status = g_lock_write_data(ctx, lockname, &data, sizeof(data));
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_LOCKED)) {
+		fprintf(stderr, "unlocked g_lock_write_data returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_WRITE,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_lock returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_write_data(ctx, lockname, &data, sizeof(data));
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_write_data failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_unlock(ctx, lockname);
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_unlock failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_dump(ctx, lockname, lock2_parser, &state);
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_dump failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	if (!state.ok) {
+		fprintf(stderr, "Could not parse data\n");
+		goto fail;
+	}
+	if (rdata != data) {
+		fprintf(stderr, "Returned %"PRIu8", expected %"PRIu8"\n",
+			rdata, data);
+		goto fail;
+	}
+
+	ret = true;
+fail:
+	TALLOC_FREE(ctx);
+	TALLOC_FREE(msg);
+	TALLOC_FREE(ev);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 5ef4d4e..2bdb079 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11478,6 +11478,7 @@ static struct {
 	{ "LOCAL-BENCH-PTHREADPOOL", run_bench_pthreadpool, 0 },
 	{ "LOCAL-PTHREADPOOL-TEVENT", run_pthreadpool_tevent, 0 },
 	{ "LOCAL-G-LOCK1", run_g_lock1, 0 },
+	{ "LOCAL-G-LOCK2", run_g_lock2, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 },
 	{NULL, NULL, 0}};
-- 
2.1.4


From 4f027586235ef11a3f64b32c518fa585ed4ce98d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 19 May 2017 16:57:00 +0200
Subject: [PATCH 15/20] g_lock: Allow lock upgrade/downgrade

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c          | 45 +++++++++++++++++++++++++++++--------------
 source3/torture/test_g_lock.c |  2 +-
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index db2f62a..9f3d6cc 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -257,7 +257,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 			       struct server_id *blocker)
 {
 	TDB_DATA data, userdata;
-	size_t i, num_locks;
+	size_t i, num_locks, my_lock;
 	struct g_lock_rec *locks, *tmp;
 	NTSTATUS status;
 	bool modified = false;
@@ -270,11 +270,27 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 		return status;
 	}
 
+	my_lock = num_locks;	/* doesn't exist yet */
+
 	for (i=0; i<num_locks; i++) {
-		if (serverid_equal(&self, &locks[i].pid)) {
-			status = NT_STATUS_INTERNAL_ERROR;
-			goto done;
+		struct g_lock_rec *lock = &locks[i];
+
+		if (serverid_equal(&self, &lock->pid)) {
+			if (lock->lock_type == type) {
+				status = NT_STATUS_WAS_LOCKED;
+				goto done;
+			}
+			my_lock = i;
+			break;
 		}
+	}
+
+	for (i=0; i<num_locks; i++) {
+
+		if (i == my_lock) {
+			continue;
+		}
+
 		if (g_lock_conflicts(type, locks[i].lock_type)) {
 			struct server_id pid = locks[i].pid;
 
@@ -300,18 +316,19 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 		}
 	}
 
-	tmp = talloc_realloc(talloc_tos(), locks, struct g_lock_rec,
-			     num_locks+1);
-	if (tmp == NULL) {
-		status = NT_STATUS_NO_MEMORY;
-		goto done;
+	if (my_lock >= num_locks) {
+		tmp = talloc_realloc(talloc_tos(), locks, struct g_lock_rec,
+				     num_locks+1);
+		if (tmp == NULL) {
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
+		}
+		locks = tmp;
+		my_lock = num_locks;
+		num_locks += 1;
 	}
-	locks = tmp;
 
-	ZERO_STRUCT(locks[num_locks]);
-	locks[num_locks].pid = self;
-	locks[num_locks].lock_type = type;
-	num_locks += 1;
+	locks[my_lock] = (struct g_lock_rec){ .pid = self, .lock_type = type };
 	modified = true;
 
 	status = NT_STATUS_OK;
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index 5a1ee60..ac45174 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -75,7 +75,7 @@ bool run_g_lock1(int dummy)
 
 	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
 			     (struct timeval) { .tv_sec = 1 });
-	if (!NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)) {
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_WAS_LOCKED)) {
 		fprintf(stderr, "Double lock got %s\n",
 			nt_errstr(status));
 		goto fail;
-- 
2.1.4


From fe038dd501e77035c9e89f41943a0cc7df5e8572 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 19 May 2017 17:02:08 +0200
Subject: [PATCH 16/20] torture3: Test lock upgrade/downgrade

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py     |   1 +
 source3/torture/proto.h       |   1 +
 source3/torture/test_g_lock.c | 120 ++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c     |   1 +
 4 files changed, 123 insertions(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 7ad75b7..07af495 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -150,6 +150,7 @@ local_tests = [
     "LOCAL-DBWRAP-WATCH2",
     "LOCAL-G-LOCK1",
     "LOCAL-G-LOCK2",
+    "LOCAL-G-LOCK3",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index df2a241..046e9f9 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -126,5 +126,6 @@ bool run_oplock_cancel(int dummy);
 bool run_pthreadpool_tevent(int dummy);
 bool run_g_lock1(int dummy);
 bool run_g_lock2(int dummy);
+bool run_g_lock3(int dummy);
 
 #endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index ac45174..154b1c1 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -22,6 +22,7 @@
 #include "system/filesys.h"
 #include "g_lock.h"
 #include "messages.h"
+#include "lib/util/server_id.h"
 
 static bool get_g_lock_ctx(TALLOC_CTX *mem_ctx,
 			   struct tevent_context **ev,
@@ -198,3 +199,122 @@ fail:
 	TALLOC_FREE(ev);
 	return ret;
 }
+
+struct lock3_parser_state {
+	struct server_id self;
+	enum g_lock_type lock_type;
+	bool ok;
+};
+
+static void lock3_parser(const struct g_lock_rec *locks,
+			 size_t num_locks,
+			 const uint8_t *data,
+			 size_t datalen,
+			 void *private_data)
+{
+	struct lock3_parser_state *state = private_data;
+
+	if (datalen != 0) {
+		fprintf(stderr, "datalen=%zu\n", datalen);
+		return;
+	}
+	if (num_locks != 1) {
+		fprintf(stderr, "num_locks=%zu\n", num_locks);
+		return;
+	}
+	if (locks[0].lock_type != state->lock_type) {
+		fprintf(stderr, "found type %d, expected %d\n",
+			(int)locks[0].lock_type, (int)state->lock_type);
+		return;
+	}
+	if (!server_id_equal(&locks[0].pid, &state->self)) {
+		struct server_id_buf tmp1, tmp2;
+		fprintf(stderr, "found pid %s, expected %s\n",
+			server_id_str_buf(locks[0].pid, &tmp1),
+			server_id_str_buf(state->self, &tmp2));
+		return;
+	}
+
+	state->ok = true;
+}
+
+/*
+ * Test lock upgrade/downgrade
+ */
+
+bool run_g_lock3(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock3";
+	struct lock3_parser_state state;
+	NTSTATUS status;
+	bool ret = false;
+	bool ok;
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		goto fail;
+	}
+
+	state.self = messaging_server_id(msg);
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_lock returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_WAS_LOCKED)) {
+		fprintf(stderr, "g_lock_lock returned %s, expected %s\n",
+			nt_errstr(status), nt_errstr(NT_STATUS_WAS_LOCKED));
+		goto fail;
+	}
+
+	state.lock_type = G_LOCK_READ;
+	state.ok = false;
+
+	status = g_lock_dump(ctx, lockname, lock3_parser, &state);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_OK)) {
+		fprintf(stderr, "g_lock_dump returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+	if (!state.ok) {
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_WRITE,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_lock returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	state.lock_type = G_LOCK_WRITE;
+	state.ok = false;
+
+	status = g_lock_dump(ctx, lockname, lock3_parser, &state);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_OK)) {
+		fprintf(stderr, "g_lock_dump returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+	if (!state.ok) {
+		goto fail;
+	}
+
+
+	ret = true;
+fail:
+	TALLOC_FREE(ctx);
+	TALLOC_FREE(msg);
+	TALLOC_FREE(ev);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 2bdb079..4661da9 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11479,6 +11479,7 @@ static struct {
 	{ "LOCAL-PTHREADPOOL-TEVENT", run_pthreadpool_tevent, 0 },
 	{ "LOCAL-G-LOCK1", run_g_lock1, 0 },
 	{ "LOCAL-G-LOCK2", run_g_lock2, 0 },
+	{ "LOCAL-G-LOCK3", run_g_lock3, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 },
 	{NULL, NULL, 0}};
-- 
2.1.4


From 45f586183ffc63d4e1b84eb6a1521b13f8ddd8d2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 21 May 2017 08:56:01 +0200
Subject: [PATCH 17/20] torture3: Test lock conflict and cleanup

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py     |   1 +
 source3/torture/proto.h       |   1 +
 source3/torture/test_g_lock.c | 182 ++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c     |   1 +
 4 files changed, 185 insertions(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 07af495..8a8a62d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -151,6 +151,7 @@ local_tests = [
     "LOCAL-G-LOCK1",
     "LOCAL-G-LOCK2",
     "LOCAL-G-LOCK3",
+    "LOCAL-G-LOCK4",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 046e9f9..c945d92 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -127,5 +127,6 @@ bool run_pthreadpool_tevent(int dummy);
 bool run_g_lock1(int dummy);
 bool run_g_lock2(int dummy);
 bool run_g_lock3(int dummy);
+bool run_g_lock4(int dummy);
 
 #endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index 154b1c1..98fd013 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -23,6 +23,7 @@
 #include "g_lock.h"
 #include "messages.h"
 #include "lib/util/server_id.h"
+#include "lib/util/sys_rw.h"
 
 static bool get_g_lock_ctx(TALLOC_CTX *mem_ctx,
 			   struct tevent_context **ev,
@@ -318,3 +319,184 @@ fail:
 	TALLOC_FREE(ev);
 	return ret;
 }
+
+static bool lock4_child(const char *lockname,
+			int ready_pipe, int exit_pipe)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	NTSTATUS status;
+	ssize_t n;
+	bool ok;
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		return false;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_WRITE,
+			     (struct timeval) { .tv_sec = 1 });
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "child: g_lock_lock returned %s\n",
+			nt_errstr(status));
+		return false;
+	}
+
+	n = sys_write(ready_pipe, &ok, sizeof(ok));
+	if (n != sizeof(ok)) {
+		fprintf(stderr, "child: write failed\n");
+		return false;
+	}
+
+	if (ok) {
+		n = sys_read(exit_pipe, &ok, sizeof(ok));
+		if (n != 0) {
+			fprintf(stderr, "child: read failed\n");
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static void lock4_done(struct tevent_req *subreq)
+{
+	int *done = tevent_req_callback_data_void(subreq);
+	NTSTATUS status;
+
+	status = g_lock_lock_recv(subreq);
+	TALLOC_FREE(subreq);
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "g_lock_lock_recv returned %s\n",
+			nt_errstr(status));
+		*done = -1;
+		return;
+	}
+	*done = 1;
+}
+
+static void lock4_waited(struct tevent_req *subreq)
+{
+        int *exit_pipe = tevent_req_callback_data_void(subreq);
+	pid_t child;
+	int status;
+	bool ok;
+
+	printf("waited\n");
+
+	ok = tevent_wakeup_recv(subreq);
+	TALLOC_FREE(subreq);
+	if (!ok) {
+		fprintf(stderr, "tevent_wakeup_recv failed\n");
+	}
+	close(*exit_pipe);
+
+	child = wait(&status);
+
+	printf("child %d exited with %d\n", (int)child, status);
+}
+
+/*
+ * Test a lock conflict
+ */
+
+bool run_g_lock4(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock4";
+	pid_t child;
+	int ready_pipe[2];
+	int exit_pipe[2];
+	NTSTATUS status;
+	bool ret = false;
+	struct tevent_req *req;
+	bool ok;
+	int done;
+
+	if ((pipe(ready_pipe) != 0) || (pipe(exit_pipe) != 0)) {
+		perror("pipe failed");
+		return false;
+	}
+
+	child = fork();
+
+	ok = get_g_lock_ctx(talloc_tos(), &ev, &msg, &ctx);
+	if (!ok) {
+		goto fail;
+	}
+
+	if (child == -1) {
+		perror("fork failed");
+		return false;
+	}
+
+	if (child == 0) {
+		close(ready_pipe[0]);
+		close(exit_pipe[1]);
+		ok = lock4_child(lockname, ready_pipe[1], exit_pipe[0]);
+		exit(ok ? 0 : 1);
+	}
+
+	close(ready_pipe[1]);
+	close(exit_pipe[0]);
+
+	if (sys_read(ready_pipe[0], &ok, sizeof(ok)) != sizeof(ok)) {
+		perror("read failed");
+		return false;
+	}
+
+	if (!ok) {
+		fprintf(stderr, "child returned error\n");
+		return false;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_WRITE,
+			     (struct timeval) { .tv_usec = 1 });
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
+		fprintf(stderr, "g_lock_lock returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+			     (struct timeval) { .tv_usec = 1 });
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
+		fprintf(stderr, "g_lock_lock returned %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	req = g_lock_lock_send(ev, ev, ctx, lockname, G_LOCK_WRITE);
+	if (req == NULL) {
+		fprintf(stderr, "g_lock_lock send failed\n");
+		goto fail;
+	}
+	tevent_req_set_callback(req, lock4_done, &done);
+
+	req = tevent_wakeup_send(ev, ev, timeval_current_ofs(1, 0));
+	if (req == NULL) {
+		fprintf(stderr, "tevent_wakeup_send failed\n");
+		goto fail;
+	}
+	tevent_req_set_callback(req, lock4_waited, &exit_pipe[1]);
+
+	done = 0;
+
+	while (done == 0) {
+		int tevent_ret = tevent_loop_once(ev);
+		if (tevent_ret != 0) {
+			perror("tevent_loop_once failed");
+			goto fail;
+		}
+	}
+
+	ret = true;
+fail:
+	TALLOC_FREE(ctx);
+	TALLOC_FREE(msg);
+	TALLOC_FREE(ev);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 4661da9..81d180e 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11480,6 +11480,7 @@ static struct {
 	{ "LOCAL-G-LOCK1", run_g_lock1, 0 },
 	{ "LOCAL-G-LOCK2", run_g_lock2, 0 },
 	{ "LOCAL-G-LOCK3", run_g_lock3, 0 },
+	{ "LOCAL-G-LOCK4", run_g_lock4, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 },
 	{NULL, NULL, 0}};
-- 
2.1.4


From f56f4e4a540c35ad113eb8cbfdee828295a73aaf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 22 May 2017 17:05:57 +0200
Subject: [PATCH 18/20] g_lock: Heuristically check for server existence

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/g_lock.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index 9f3d6cc..9342302 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -272,6 +272,22 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 
 	my_lock = num_locks;	/* doesn't exist yet */
 
+	if ((type == G_LOCK_READ) && (num_locks > 0)) {
+		/*
+		 * Read locks can stay around forever if the process
+		 * dies. Do a heuristic check for process existence:
+		 * Check one random process for existence. Hopefully
+		 * this will keep runaway read locks under control.
+		 */
+		i = generate_random() % num_locks;
+
+		if (!serverid_exists(&locks[i].pid)) {
+			locks[i] = locks[num_locks-1];
+			num_locks -=1;
+			modified = true;
+		}
+	}
+
 	for (i=0; i<num_locks; i++) {
 		struct g_lock_rec *lock = &locks[i];
 
-- 
2.1.4


From 8d0a325aa72ee0dcd4bec05bf666d5ffae5e1a63 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 25 May 2017 10:48:15 +0200
Subject: [PATCH 19/20] torture3: Test heuristic cleanup

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py     |   1 +
 source3/torture/proto.h       |   1 +
 source3/torture/test_g_lock.c | 142 ++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c     |   1 +
 4 files changed, 145 insertions(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 8a8a62d..965ad7e 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -152,6 +152,7 @@ local_tests = [
     "LOCAL-G-LOCK2",
     "LOCAL-G-LOCK3",
     "LOCAL-G-LOCK4",
+    "LOCAL-G-LOCK5",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index c945d92..6d5ca77 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -128,5 +128,6 @@ bool run_g_lock1(int dummy);
 bool run_g_lock2(int dummy);
 bool run_g_lock3(int dummy);
 bool run_g_lock4(int dummy);
+bool run_g_lock5(int dummy);
 
 #endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c
index 98fd013..ca37312 100644
--- a/source3/torture/test_g_lock.c
+++ b/source3/torture/test_g_lock.c
@@ -500,3 +500,145 @@ fail:
 	TALLOC_FREE(ev);
 	return ret;
 }
+
+struct lock5_parser_state {
+	size_t num_locks;
+};
+
+static void lock5_parser(const struct g_lock_rec *locks,
+			 size_t num_locks,
+			 const uint8_t *data,
+			 size_t datalen,
+			 void *private_data)
+{
+	struct lock5_parser_state *state = private_data;
+	state->num_locks = num_locks;
+}
+
+/*
+ * Test heuristic cleanup
+ */
+
+bool run_g_lock5(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct g_lock_ctx *ctx = NULL;
+	const char *lockname = "lock5";
+	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;
+
+	nprocs = 5;
+
+	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;
+	}
+
+	for (i=0; i<nprocs; i++) {
+
+		child = fork();
+
+		if (child == -1) {
+			perror("fork failed");
+			return false;
+		}
+
+		if (child == 0) {
+			TALLOC_FREE(ctx);
+			TALLOC_FREE(msg);
+			TALLOC_FREE(ev);
+
+			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);
+			}
+			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 %zu (%s)\n",
+			nread, strerror(errno));
+		return false;
+	}
+
+	close(exit_pipe[1]);
+
+	for (i=0; i<nprocs; i++) {
+		int child_status;
+		ret = waitpid(-1, &child_status, 0);
+		if (ret == -1) {
+			perror("waitpid failed");
+			return false;
+		}
+	}
+
+	for (i=0; i<nprocs; i++) {
+		struct lock5_parser_state state;
+
+		status = g_lock_dump(ctx, lockname, lock5_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 - i)) {
+			fprintf(stderr, "nlocks=%zu, expected %zu\n",
+				state.num_locks, (nprocs-i));
+			return false;
+		}
+
+		status = g_lock_lock(ctx, lockname, G_LOCK_READ,
+				     (struct timeval) { .tv_sec = 1 });
+		if (!NT_STATUS_IS_OK(status)) {
+			fprintf(stderr, "g_lock_lock failed %s\n",
+				nt_errstr(status));
+			return false;
+		}
+		status = g_lock_unlock(ctx, lockname);
+		if (!NT_STATUS_IS_OK(status)) {
+			fprintf(stderr, "g_lock_unlock failed %s\n",
+				nt_errstr(status));
+			return false;
+		}
+	}
+
+
+	return true;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 81d180e..2d4b62d 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11481,6 +11481,7 @@ static struct {
 	{ "LOCAL-G-LOCK2", run_g_lock2, 0 },
 	{ "LOCAL-G-LOCK3", run_g_lock3, 0 },
 	{ "LOCAL-G-LOCK4", run_g_lock4, 0 },
+	{ "LOCAL-G-LOCK5", run_g_lock5, 0 },
 	{ "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 },
 	{ "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 },
 	{NULL, NULL, 0}};
-- 
2.1.4


From cec9c8018205fcc6b68863b55cda51251ccd59e1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 22 May 2017 16:00:08 +0200
Subject: [PATCH 20/20] smbd: Claim version in g_lock

Protect smbd against version incompatibilities in a cluster.

At first startup smbd locks "samba_version_string" and writes its version
string. It then downgrades the lock to a read lock. Subsequent smbds check
against the version string and also keep the read lock around. If the version
does not match, we try to write our own version. But as there's a read lock,
the lock upgrade to write lock will fail due the read lock being around. So as
long as there's one smbd with this read lock, no other version of smbd will be
able to start.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 .../smbdotconf/misc/allowunsafeclusterupgrade.xml  |  16 +++
 source3/smbd/server.c                              | 114 +++++++++++++++++++++
 2 files changed, 130 insertions(+)
 create mode 100644 docs-xml/smbdotconf/misc/allowunsafeclusterupgrade.xml

diff --git a/docs-xml/smbdotconf/misc/allowunsafeclusterupgrade.xml b/docs-xml/smbdotconf/misc/allowunsafeclusterupgrade.xml
new file mode 100644
index 0000000..02398ff
--- /dev/null
+++ b/docs-xml/smbdotconf/misc/allowunsafeclusterupgrade.xml
@@ -0,0 +1,16 @@
+<samba:parameter name="allow unsafe cluster upgrade"
+                 context="G"
+                 type="boolean"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+	<para>If set to no (the default), smbd checks at startup if
+	other smbd versions are running in the cluster and refuses to
+	start if so. This is done to protect data corruption in
+	internal data structures due to incompatible Samba versions
+	running concurrently in the same cluster. Setting this
+	parameter to <value type="example">yes</value> disables this
+	safety check.
+	</para>
+</description>
+<value type="default">no</value>
+</samba:parameter>
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 494e188..4883c4e 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -53,6 +53,7 @@
 #include "smbd/smbd_cleanupd.h"
 #include "lib/util/sys_rw.h"
 #include "cleanupdb.h"
+#include "g_lock.h"
 
 #ifdef CLUSTER_SUPPORT
 #include "ctdb_protocol.h"
@@ -1442,6 +1443,110 @@ static void smbd_parent_sig_hup_handler(struct tevent_context *ev,
 	printing_subsystem_update(parent->ev_ctx, parent->msg_ctx, true);
 }
 
+struct smbd_claim_version_state {
+	TALLOC_CTX *mem_ctx;
+	char *version;
+};
+
+static void smbd_claim_version_parser(const struct g_lock_rec *locks,
+				      size_t num_locks,
+				      const uint8_t *data,
+				      size_t datalen,
+				      void *private_data)
+{
+	struct smbd_claim_version_state *state = private_data;
+
+	if (datalen == 0) {
+		state->version = NULL;
+		return;
+	}
+	if (data[datalen-1] != '\0') {
+		DBG_WARNING("Invalid samba version\n");
+		dump_data(DBGLVL_WARNING, data, datalen);
+		state->version = NULL;
+		return;
+	}
+	state->version = talloc_strdup(state->mem_ctx, (const char *)data);
+}
+
+static NTSTATUS smbd_claim_version(struct messaging_context *msg,
+				   const char *version)
+{
+	const char *name = "samba_version_string";
+	struct smbd_claim_version_state state;
+	struct g_lock_ctx *ctx;
+	NTSTATUS status;
+
+	ctx = g_lock_ctx_init(msg, msg);
+	if (ctx == NULL) {
+		DBG_WARNING("g_lock_ctx_init failed\n");
+		return NT_STATUS_UNSUCCESSFUL;
+	}
+
+	status = g_lock_lock(ctx, name, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 60 });
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("g_lock_lock(G_LOCK_READ) failed: %s\n",
+			    nt_errstr(status));
+		TALLOC_FREE(ctx);
+		return status;
+	}
+
+	state = (struct smbd_claim_version_state) { .mem_ctx = ctx };
+
+	status = g_lock_dump(ctx, name, smbd_claim_version_parser, &state);
+	if (!NT_STATUS_IS_OK(status) &&
+	    !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+		DBG_ERR("Could not read samba_version_string\n");
+		g_lock_unlock(ctx, name);
+		TALLOC_FREE(ctx);
+		return status;
+	}
+
+	if ((state.version != NULL) && (strcmp(version, state.version) == 0)) {
+		/*
+		 * Leave the read lock for us around. Someone else already
+		 * set the version correctly
+		 */
+		TALLOC_FREE(ctx);
+		return NT_STATUS_OK;
+	}
+
+	status = g_lock_lock(ctx, name, G_LOCK_WRITE,
+			     (struct timeval) { .tv_sec = 60 });
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("g_lock_lock(G_LOCK_WRITE) failed: %s\n",
+			    nt_errstr(status));
+		DBG_ERR("smbd %s already running, refusing to start "
+			"version %s\n", state.version, version);
+		TALLOC_FREE(ctx);
+		return NT_STATUS_SXS_VERSION_CONFLICT;
+	}
+
+	status = g_lock_write_data(ctx, name, (const uint8_t *)version,
+				   strlen(version)+1);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("g_lock_write_data failed: %s\n",
+			    nt_errstr(status));
+		TALLOC_FREE(ctx);
+		return status;
+	}
+
+	status = g_lock_lock(ctx, name, G_LOCK_READ,
+			     (struct timeval) { .tv_sec = 60 });
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("g_lock_lock(G_LOCK_READ) failed: %s\n",
+			    nt_errstr(status));
+		TALLOC_FREE(ctx);
+		return status;
+	}
+
+	/*
+	 * Leave "ctx" dangling so that g_lock.tdb keeps opened.
+	 */
+	return NT_STATUS_OK;
+}
+
 /****************************************************************************
  main program.
 ****************************************************************************/
@@ -1905,6 +2010,15 @@ extern void build_options(bool screen);
 		exit_daemon("Samba cannot init global open", map_errno_from_nt_status(status));
 	}
 
+	if (lp_clustering() && !lp_allow_unsafe_cluster_upgrade()) {
+		status = smbd_claim_version(msg_ctx, samba_version_string());
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_WARNING("Could not claim version: %s\n",
+				    nt_errstr(status));
+			return -1;
+		}
+	}
+
 	/* This MUST be done before start_epmd() because otherwise
 	 * start_epmd() forks and races against dcesrv_ep_setup() to
 	 * call directory_create_or_exist() */
-- 
2.1.4



More information about the samba-technical mailing list