Attachment 17487 Details for Bug 15148 – Patches for v4-17-test

[patch] Patches for v4-17-test

bfixes-tmp417.txt (text/plain), 12.42 KB, created by Stefan Metzmacher on 2022-08-22 06:06:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-08-22 06:06:16 UTC

Size: 12.42 KB

patch

obsolete

>From 322c0b86cd902af6cd42aa7f99b18c672a528a90 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 15 Aug 2022 10:49:13 +0200
>Subject: [PATCH 1/3] s3:smbd: share_mode_flags_set() takes SMB2_LEASE_* values
>
>We currently only ever pass SMB2_LEASE_READ and both
>have the same value of 0x1, so for now it's only cosmetic,
>but that will change soon.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 7592aad4d7a84d0ac66a156a22af3ad77803e55c)
>---
> source3/smbd/open.c        | 2 +-
> source3/smbd/smb2_oplock.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 803c0fbd347a..992656563bf9 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -2789,7 +2789,7 @@ grant:
> 	if (granted & SMB2_LEASE_READ) {
> 		uint32_t acc, sh, ls;
> 		share_mode_flags_get(lck, &acc, &sh, &ls);
>-		ls |= SHARE_MODE_LEASE_READ;
>+		ls |= SMB2_LEASE_READ;
> 		share_mode_flags_set(lck, acc, sh, ls, NULL);
> 	}
> 
>diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c
>index 367b0fd3191a..a555d3ae4fe2 100644
>--- a/source3/smbd/smb2_oplock.c
>+++ b/source3/smbd/smb2_oplock.c
>@@ -1310,7 +1310,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
> 		 */
> 		uint32_t acc, sh, ls;
> 		share_mode_flags_get(lck, &acc, &sh, &ls);
>-		ls &= ~SHARE_MODE_LEASE_READ;
>+		ls &= ~SMB2_LEASE_READ;
> 		share_mode_flags_set(lck, acc, sh, ls, NULL);
> 	}
> 
>-- 
>2.34.1
>
>
>From 4db4bda518cda872fad673255a4113c6b8962eb8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 17 Aug 2022 17:07:08 +0200
>Subject: [PATCH 2/3] s4:torture/smb2: add smb2.lease.v[1,2]_bug_15148
>
>This demonstrates the bug that happens with a
>write to a file handle holding an R lease,
>while there are other openers without any lease.
>
>When one of the other openers writes to the file,
>the R lease of the only lease holder isn't broken to NONE.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 9e5ff607eb1b9c45c8836d3cff9d51b418740b87)
>---
> selftest/knownfail.d/lease_bug_15148 |   2 +
> source4/torture/smb2/lease.c         | 208 +++++++++++++++++++++++++++
> 2 files changed, 210 insertions(+)
> create mode 100644 selftest/knownfail.d/lease_bug_15148
>
>diff --git a/selftest/knownfail.d/lease_bug_15148 b/selftest/knownfail.d/lease_bug_15148
>new file mode 100644
>index 000000000000..e06a3d1b030e
>--- /dev/null
>+++ b/selftest/knownfail.d/lease_bug_15148
>@@ -0,0 +1,2 @@
>+^samba3.smb2.lease.v1_bug15148
>+^samba3.smb2.lease.v2_bug15148
>diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
>index 43b418c5acf4..a2c354dc02ad 100644
>--- a/source4/torture/smb2/lease.c
>+++ b/source4/torture/smb2/lease.c
>@@ -4556,6 +4556,210 @@ done:
> 	return ret;
> }
> 
>+static bool test_lease_v1_bug_15148(struct torture_context *tctx,
>+				    struct smb2_tree *tree)
>+{
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	struct smb2_create io1;
>+	struct smb2_create io2;
>+	struct smb2_lease ls1;
>+	struct smb2_lease ls2;
>+	struct smb2_handle h1 = {{0}};
>+	struct smb2_handle h2 = {{0}};
>+	struct smb2_write w;
>+	NTSTATUS status;
>+	const char *fname = "lease_v1_bug_15148.dat";
>+	bool ret = true;
>+	uint32_t caps;
>+
>+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
>+	if (!(caps & SMB2_CAP_LEASING)) {
>+		torture_skip(tctx, "leases are not supported");
>+	}
>+
>+	tree->session->transport->lease.handler = torture_lease_handler;
>+	tree->session->transport->lease.private_data = tree;
>+	tree->session->transport->oplock.handler = torture_oplock_handler;
>+	tree->session->transport->oplock.private_data = tree;
>+
>+	smb2_util_unlink(tree, fname);
>+
>+	torture_reset_lease_break_info(tctx, &lease_break_info);
>+
>+	/* Grab R lease over connection 1a */
>+	smb2_lease_create(&io1, &ls1, false, fname, LEASE1, smb2_util_lease_state("R"));
>+	status = smb2_create(tree, mem_ctx, &io1);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h1 = io1.out.file.handle;
>+	CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	CHECK_LEASE(&io1, "R", true, LEASE1, 0);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	/* Contend with LEASE2. */
>+	smb2_lease_create(&io2, &ls2, false, fname, LEASE2, smb2_util_lease_state("R"));
>+	status = smb2_create(tree, mem_ctx, &io2);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h2 = io2.out.file.handle;
>+	CHECK_CREATED(&io2, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
>+	CHECK_LEASE(&io2, "R", true, LEASE2, 0);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h1;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'o', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	ls2.lease_epoch += 1;
>+	CHECK_BREAK_INFO("R", "", LEASE2);
>+
>+	torture_reset_lease_break_info(tctx, &lease_break_info);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h1;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'O', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h2;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'o', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	ls1.lease_epoch += 1;
>+	CHECK_BREAK_INFO("R", "", LEASE1);
>+
>+ done:
>+	smb2_util_close(tree, h1);
>+	smb2_util_close(tree, h2);
>+
>+	smb2_util_unlink(tree, fname);
>+
>+	talloc_free(mem_ctx);
>+
>+	return ret;
>+}
>+
>+static bool test_lease_v2_bug_15148(struct torture_context *tctx,
>+				    struct smb2_tree *tree)
>+{
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	struct smb2_create io1;
>+	struct smb2_create io2;
>+	struct smb2_lease ls1;
>+	struct smb2_lease ls2;
>+	struct smb2_handle h1 = {{0}};
>+	struct smb2_handle h2 = {{0}};
>+	struct smb2_write w;
>+	NTSTATUS status;
>+	const char *fname = "lease_v2_bug_15148.dat";
>+	bool ret = true;
>+	uint32_t caps;
>+	enum protocol_types protocol;
>+
>+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
>+	if (!(caps & SMB2_CAP_LEASING)) {
>+		torture_skip(tctx, "leases are not supported");
>+	}
>+
>+	protocol = smbXcli_conn_protocol(tree->session->transport->conn);
>+	if (protocol < PROTOCOL_SMB3_00) {
>+		torture_skip(tctx, "v2 leases are not supported");
>+	}
>+
>+	tree->session->transport->lease.handler = torture_lease_handler;
>+	tree->session->transport->lease.private_data = tree;
>+	tree->session->transport->oplock.handler = torture_oplock_handler;
>+	tree->session->transport->oplock.private_data = tree;
>+
>+	smb2_util_unlink(tree, fname);
>+
>+	torture_reset_lease_break_info(tctx, &lease_break_info);
>+
>+	/* Grab R lease over connection 1a */
>+	smb2_lease_v2_create(&io1, &ls1, false, fname, LEASE1, NULL,
>+			     smb2_util_lease_state("R"), 0x4711);
>+	status = smb2_create(tree, mem_ctx, &io1);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h1 = io1.out.file.handle;
>+	CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	ls1.lease_epoch += 1;
>+	CHECK_LEASE_V2(&io1, "R", true, LEASE1,
>+		       0, 0, ls1.lease_epoch);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	/* Contend with LEASE2. */
>+	smb2_lease_v2_create(&io2, &ls2, false, fname, LEASE2, NULL,
>+			     smb2_util_lease_state("R"), 0x11);
>+	status = smb2_create(tree, mem_ctx, &io2);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h2 = io2.out.file.handle;
>+	CHECK_CREATED(&io2, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
>+	ls2.lease_epoch += 1;
>+	CHECK_LEASE_V2(&io2, "R", true, LEASE2,
>+		       0, 0, ls2.lease_epoch);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h1;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'o', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	ls2.lease_epoch += 1;
>+	CHECK_BREAK_INFO_V2(tree->session->transport,
>+			    "R", "", LEASE2, ls2.lease_epoch);
>+
>+	torture_reset_lease_break_info(tctx, &lease_break_info);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h1;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'O', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	CHECK_NO_BREAK(tctx);
>+
>+	ZERO_STRUCT(w);
>+	w.in.file.handle = h2;
>+	w.in.offset      = 0;
>+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
>+	memset(w.in.data.data, 'o', w.in.data.length);
>+	status = smb2_write(tree, &w);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+
>+	ls1.lease_epoch += 1;
>+	CHECK_BREAK_INFO_V2(tree->session->transport,
>+			    "R", "", LEASE1, ls1.lease_epoch);
>+
>+ done:
>+	smb2_util_close(tree, h1);
>+	smb2_util_close(tree, h2);
>+
>+	smb2_util_unlink(tree, fname);
>+
>+	talloc_free(mem_ctx);
>+
>+	return ret;
>+}
>+
> struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> {
> 	struct torture_suite *suite =
>@@ -4604,6 +4808,10 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> 				test_lease_duplicate_create);
> 	torture_suite_add_1smb2_test(suite, "duplicate_open",
> 				test_lease_duplicate_open);
>+	torture_suite_add_1smb2_test(suite, "v1_bug15148",
>+				test_lease_v1_bug_15148);
>+	torture_suite_add_1smb2_test(suite, "v2_bug15148",
>+				test_lease_v2_bug_15148);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
> 
>-- 
>2.34.1
>
>
>From 9036b7e636036e8b0de69acf647c621422b2f151 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 15 Aug 2022 22:45:17 +0200
>Subject: [PATCH 3/3] s3:smbd: only clear LEASE_READ if there's no read lease
> is left
>
>If contend_level2_oplocks_begin_default() skips break it's
>own lease, we should not clear SHARE_MODE_LEASE_READ
>in share_mode_data->flags.
>
>Otherwise that lease won't see any lease break notifications
>for writes from other clients (file handles not using the same lease
>key).
>
>So we need to count the number existing read leases (including
>the one with the same lease key) in order to know it's
>safe to clear SMB2_LEASE_READ/SHARE_MODE_LEASE_READ.
>
>Otherwise the next run (likely from another client)
>will get the wrong result from file_has_read_lease().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Thu Aug 18 19:41:33 UTC 2022 on sn-devel-184
>
>(cherry picked from commit 96e2a82760ea06a89b7387b5cd3e864732afded3)
>---
> selftest/knownfail.d/lease_bug_15148 |  2 --
> source3/smbd/smb2_oplock.c           | 11 ++++++-----
> 2 files changed, 6 insertions(+), 7 deletions(-)
> delete mode 100644 selftest/knownfail.d/lease_bug_15148
>
>diff --git a/selftest/knownfail.d/lease_bug_15148 b/selftest/knownfail.d/lease_bug_15148
>deleted file mode 100644
>index e06a3d1b030e..000000000000
>--- a/selftest/knownfail.d/lease_bug_15148
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba3.smb2.lease.v1_bug15148
>-^samba3.smb2.lease.v2_bug15148
>diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c
>index a555d3ae4fe2..1f143840b341 100644
>--- a/source3/smbd/smb2_oplock.c
>+++ b/source3/smbd/smb2_oplock.c
>@@ -1141,7 +1141,7 @@ struct break_to_none_state {
> 	struct file_id id;
> 	struct smb2_lease_key lease_key;
> 	struct GUID client_guid;
>-	size_t num_broken;
>+	size_t num_read_leases;
> };
> 
> static bool do_break_lease_to_none(struct share_mode_entry *e,
>@@ -1175,6 +1175,8 @@ static bool do_break_lease_to_none(struct share_mode_entry *e,
> 		return false;
> 	}
> 
>+	state->num_read_leases += 1;
>+
> 	our_own = smb2_lease_equal(&state->client_guid,
> 				   &state->lease_key,
> 				   &e->client_guid,
>@@ -1190,8 +1192,6 @@ static bool do_break_lease_to_none(struct share_mode_entry *e,
> 
> 	send_break_to_none(state->sconn->msg_ctx, &state->id, e);
> 
>-	state->num_broken += 1;
>-
> 	return false;
> }
> 
>@@ -1225,11 +1225,12 @@ static bool do_break_oplock_to_none(struct share_mode_entry *e,
> 		return false;
> 	}
> 
>+	state->num_read_leases += 1;
>+
> 	/* Paranoia .... */
> 	SMB_ASSERT(!EXCLUSIVE_OPLOCK_TYPE(e->op_type));
> 
> 	send_break_to_none(state->sconn->msg_ctx, &state->id, e);
>-	state->num_broken += 1;
> 
> 	return false;
> }
>@@ -1303,7 +1304,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
> 		DBG_WARNING("share_mode_forall_entries failed\n");
> 	}
> 
>-	if (state.num_broken == 0) {
>+	if (state.num_read_leases == 0) {
> 		/*
> 		 * Lazy update here. It might be that the read lease
> 		 * has gone in the meantime.
>-- 
>2.34.1
>

Flags:

slow: review+

Actions: View

Attachments on bug 15148: 17485 | 17486 | 17487