[PATCH] Small fixes for dbwrap_watch

vl at samba.org vl at samba.org
Fri Apr 28 12:38:36 UTC 2017


Hi!

Review appreciated!

Thanks, Volker
-------------- next part --------------
From d1736e991b132d3825c4d9e6c33ae604e298fea1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 28 Apr 2017 13:41:30 +0200
Subject: [PATCH 1/4] selftest: Make sure that LOCAL-DBWRAP-WATCH1 is run in
 make test

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 9bb7903..7a1359f 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -146,6 +146,7 @@ local_tests = [
     "LOCAL-MESSAGING-FDPASS2b",
     "LOCAL-PTHREADPOOL-TEVENT",
     "LOCAL-CANONICALIZE-PATH",
+    "LOCAL-DBWRAP-WATCH1",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
-- 
2.1.4


From cdf663503fcecc99bad15cfad687fc989f2d5722 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 28 Apr 2017 13:45:47 +0200
Subject: [PATCH 2/4] torture3: In LOCAL-DBWRAP-WATCH1, open tdb with
 CLEAR_IF_FIRST

Just make sure we start with fresh data

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

diff --git a/source3/torture/test_dbwrap_watch.c b/source3/torture/test_dbwrap_watch.c
index d3eac6f..8421bf1 100644
--- a/source3/torture/test_dbwrap_watch.c
+++ b/source3/torture/test_dbwrap_watch.c
@@ -48,7 +48,7 @@ bool run_dbwrap_watch1(int dummy)
 		fprintf(stderr, "messaging_init failed\n");
 		goto fail;
 	}
-	backend = db_open(msg, "test_watch.tdb", 0, TDB_DEFAULT,
+	backend = db_open(msg, "test_watch.tdb", 0, TDB_CLEAR_IF_FIRST,
 			  O_CREAT|O_RDWR, 0644, DBWRAP_LOCK_ORDER_1,
 			  DBWRAP_FLAG_NONE);
 	if (backend == NULL) {
-- 
2.1.4


From 780633afa14277d64634273cc542d313e615dc8e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 15 Mar 2017 16:54:34 +0100
Subject: [PATCH 3/4] dbwrap_watch: Protect against corrupt records

If locking.tdb contains invalid records, "get_file_infos" called from directory
enumeration crashes in Samba 4.4. The reason is that if "dbwrap_watched_parse"
returns -1 due to record corruption, dbwrap_watched_parse_record returns
NT_STATUS_OK without having called the parse function. Before 66cba9939b76f
this led to "lck->data" to be uninitialized data, so smbd 4.4 would crash in
this case.  After 66cba9939b76f we implicitly initialize "state.lck" to NULL,
so we don't have this particular problem anymore

Apply the fix in master too, returning NT_STATUS_OK from parse_record without
having called the parser could lead to bugs in other cases too.

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

diff --git a/source3/lib/dbwrap/dbwrap_watch.c b/source3/lib/dbwrap/dbwrap_watch.c
index 6057bf4..585010f 100644
--- a/source3/lib/dbwrap/dbwrap_watch.c
+++ b/source3/lib/dbwrap/dbwrap_watch.c
@@ -521,7 +521,10 @@ static void dbwrap_watched_parse_record_parser(TDB_DATA key, TDB_DATA data,
 
 	num_watchers = dbwrap_watched_parse(data, NULL, 0, &state->deleted,
 					    &userdata);
-	if ((num_watchers == -1) || state->deleted) {
+	if (num_watchers == -1) {
+		state->deleted = true;
+	}
+	if (state->deleted) {
 		return;
 	}
 	state->parser(key, userdata, state->private_data);
-- 
2.1.4


From 303bca9b97153a22723656781d563544012cb7a0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 28 Apr 2017 13:58:48 +0200
Subject: [PATCH 4/4] torture3: Make sure dbwrap_parse_record returns NOT_FOUND
 for invalid watchers data

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

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 7a1359f..1d02bd2 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -147,6 +147,7 @@ local_tests = [
     "LOCAL-PTHREADPOOL-TEVENT",
     "LOCAL-CANONICALIZE-PATH",
     "LOCAL-DBWRAP-WATCH1",
+    "LOCAL-DBWRAP-WATCH2",
     "LOCAL-hex_encode_buf",
     "LOCAL-remove_duplicate_addrs2"]
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index da0c69f..129c05d 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -109,6 +109,7 @@ bool run_cleanup4(int dummy);
 bool run_notify_bench2(int dummy);
 bool run_notify_bench3(int dummy);
 bool run_dbwrap_watch1(int dummy);
+bool run_dbwrap_watch2(int dummy);
 bool run_idmap_tdb_common_test(int dummy);
 bool run_local_dbwrap_ctdb(int dummy);
 bool run_qpathinfo_bufsize(int dummy);
diff --git a/source3/torture/test_dbwrap_watch.c b/source3/torture/test_dbwrap_watch.c
index 8421bf1..f775dc7 100644
--- a/source3/torture/test_dbwrap_watch.c
+++ b/source3/torture/test_dbwrap_watch.c
@@ -107,3 +107,71 @@ fail:
 	TALLOC_FREE(ev);
 	return ret;
 }
+
+/*
+ * Make sure dbwrap_parse_record does not return NT_STATUS_OK on
+ * invalid data
+ */
+
+bool run_dbwrap_watch2(int dummy)
+{
+	struct tevent_context *ev = NULL;
+	struct messaging_context *msg = NULL;
+	struct db_context *backend = NULL;
+	struct db_context *db = NULL;
+	const char *keystr = "key";
+	TDB_DATA key = string_term_tdb_data(keystr);
+	struct db_record *rec = NULL;
+	NTSTATUS status;
+	bool ret = false;
+
+	ev = samba_tevent_context_init(talloc_tos());
+	if (ev == NULL) {
+		fprintf(stderr, "tevent_context_init failed\n");
+		goto fail;
+	}
+	msg = messaging_init(ev, ev);
+	if (msg == NULL) {
+		fprintf(stderr, "messaging_init failed\n");
+		goto fail;
+	}
+	backend = db_open(msg, "test_watch.tdb", 0, TDB_CLEAR_IF_FIRST,
+			  O_CREAT|O_RDWR, 0644, DBWRAP_LOCK_ORDER_1,
+			  DBWRAP_FLAG_NONE);
+	if (backend == NULL) {
+		fprintf(stderr, "db_open failed: %s\n", strerror(errno));
+		goto fail;
+	}
+
+	/*
+	 * Store invalid data (from the dbwrap_watch point of view)
+	 * directly into the backend database
+	 */
+	status = dbwrap_store_uint32_bystring(backend, keystr, UINT32_MAX);
+	if (!NT_STATUS_IS_OK(status)) {
+		fprintf(stderr, "dbwrap_store_uint32_bystring failed: %s\n",
+			nt_errstr(status));
+		goto fail;
+	}
+
+	db = db_open_watched(ev, backend, msg);
+	if (db == NULL) {
+		fprintf(stderr, "db_open_watched failed\n");
+		goto fail;
+	}
+
+	status = dbwrap_parse_record(db, key, NULL, NULL);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+		fprintf(stderr, "dbwrap_parse_record returned %s, expected "
+			"NT_STATUS_NOT_FOUND\n", nt_errstr(status));
+		goto fail;
+	}
+
+	ret = true;
+fail:
+	TALLOC_FREE(rec);
+	TALLOC_FREE(db);
+	TALLOC_FREE(msg);
+	TALLOC_FREE(ev);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 0d9a653..bdcf1e1 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -11448,6 +11448,7 @@ static struct {
 	{ "LOCAL-GENCACHE", run_local_gencache, 0},
 	{ "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0},
 	{ "LOCAL-DBWRAP-WATCH1", run_dbwrap_watch1, 0 },
+	{ "LOCAL-DBWRAP-WATCH2", run_dbwrap_watch2, 0 },
 	{ "LOCAL-MESSAGING-READ1", run_messaging_read1, 0 },
 	{ "LOCAL-MESSAGING-READ2", run_messaging_read2, 0 },
 	{ "LOCAL-MESSAGING-READ3", run_messaging_read3, 0 },
-- 
2.1.4



More information about the samba-technical mailing list