[PATCH] Small fixes for dbwrap_watch
Jeremy Allison
jra at samba.org
Sat Apr 29 03:40:43 UTC 2017
On Fri, Apr 28, 2017 at 02:41:27PM +0200, vl--- via samba-technical wrote:
> On Fri, Apr 28, 2017 at 02:38:36PM +0200, vl at samba.org wrote:
> > Review appreciated!
>
> Wrong version sent, sorry.
>
> Review appreciated!
So this LGTM, and I pushed it - but the autobuild fails right
at the very end with:
Waf: Leaving directory `/memdisk/jra/a/b839085/samba/bin'
'install' finished successfully (1m16.849s)
The tree has 1 new uncommitted files!!!
git clean -n
Would remove test_watch.tdb
This is because the LOCAL-DBWRAP-WATCH1 and
LOCAL-DBWRAP-WATCH2 tests leave the test_watch.tdb
file still existing in the directory on success.
So here is a slightly modified version, that
adds a:
(void)unlink("test_watch.tdb");
call just before returning success, which should
fix this issue.
On failure I leave the bad file around so
we can examine for debugging purposes.
Volker, if you agree - please push !
Cheers,
Jeremy.
-------------- next part --------------
>From 519e05d3136ef892ee7555716ca6b88ce0f4b3f7 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>
Reviewed-by: Jeremy Allison <jra 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 9bb7903ac32..7a1359f4bb8 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.11.0
>From b4974a6e74ff40157081a3b0aed9011cd8e67d9c 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
Also ensure we delete the temp tdb file on success.
Just make sure we start with fresh data
Signed-off-by: Volker Lendecke <vl at samba.org>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/torture/test_dbwrap_watch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/torture/test_dbwrap_watch.c b/source3/torture/test_dbwrap_watch.c
index d3eac6f26f8..88e0e1cd9fe 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) {
@@ -98,6 +98,7 @@ bool run_dbwrap_watch1(int dummy)
goto fail;
}
+ (void)unlink("test_watch.tdb");
ret = true;
fail:
TALLOC_FREE(req);
--
2.11.0
>From b29cc95af896b9cc34064261d9ab1ea239353858 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>
Reviewed-by: Jeremy Allison <jra 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 6057bf4caad..585010f3dda 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.11.0
>From d7fba0929ff1456147a6d98057213cd95f28b342 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>
Reviewed-by: Jeremy Allison <jra 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 7a1359f4bb8..1d02bd25ef5 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 da0c69f4f69..129c05d7e7a 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 88e0e1cd9fe..5aa0430b111 100644
--- a/source3/torture/test_dbwrap_watch.c
+++ b/source3/torture/test_dbwrap_watch.c
@@ -108,3 +108,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);
+ 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;
+ }
+ backend = NULL; /* open_watch talloc_moves backend */
+
+ 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;
+ }
+
+ (void)unlink("test_watch.tdb");
+ ret = true;
+fail:
+ TALLOC_FREE(db);
+ TALLOC_FREE(msg);
+ TALLOC_FREE(ev);
+ return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 0d9a653cf20..bdcf1e13f17 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.11.0
More information about the samba-technical
mailing list