Attachment 17169 Details for Bug 14737 – git-am fix for 4.16.rcNext, 4.15.next.

[patch] git-am fix for 4.16.rcNext, 4.15.next.

bug-14737-4.16 (text/plain), 9.82 KB, created by Jeremy Allison on 2022-02-18 20:41:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2022-02-18 20:41:03 UTC

Size: 9.82 KB

patch

obsolete

>From 15ec6b6662747a4e8e79293a896c380550396151 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Thu, 17 Feb 2022 09:58:27 -0800
>Subject: [PATCH 1/3] s4: torture: Add new SMB2 lease test
> test_lease_duplicate_create().
>
>Checks we return INVALID_PARAMETER when trying to create a
>new file with a duplicate lease key on the same share.
>
>Checked against Windows10. Samba already passes this
>but we didn't have a test before.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: David Mulder <dmulder@suse.com>
>(cherry picked from commit bf22548d11fe67ea3f4ec10dff81773d626e4703)
>---
> source4/torture/smb2/lease.c | 54 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
>diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
>index 2da320483fe..30101c96de5 100644
>--- a/source4/torture/smb2/lease.c
>+++ b/source4/torture/smb2/lease.c
>@@ -4436,6 +4436,58 @@ done:
> 	return ret;
> }
> 
>+static bool test_lease_duplicate_create(struct torture_context *tctx,
>+				   struct smb2_tree *tree)
>+{
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	struct smb2_create io;
>+	struct smb2_lease ls;
>+	struct smb2_handle h1 = {{0}};
>+	struct smb2_handle h2 = {{0}};
>+	NTSTATUS status;
>+	const char *fname1 = "duplicate_create1.dat";
>+	const char *fname2 = "duplicate_create2.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");
>+	}
>+
>+	/* Ensure files don't exist. */
>+	smb2_util_unlink(tree, fname1);
>+	smb2_util_unlink(tree, fname2);
>+
>+	/* Create file1 - LEASE1 key. */
>+	smb2_lease_create(&io, &ls, false, fname1, LEASE1,
>+			  smb2_util_lease_state("RWH"));
>+	status = smb2_create(tree, mem_ctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h1 = io.out.file.handle;
>+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
>+
>+	/*
>+	 * Create file2 with the same LEASE1 key - this should fail with.
>+	 * INVALID_PARAMETER.
>+	 */
>+	smb2_lease_create(&io, &ls, false, fname2, LEASE1,
>+			  smb2_util_lease_state("RWH"));
>+	status = smb2_create(tree, mem_ctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER);
>+	smb2_util_close(tree, h1);
>+
>+done:
>+	smb2_util_close(tree, h2);
>+	smb2_util_close(tree, h1);
>+	smb2_util_unlink(tree, fname1);
>+	smb2_util_unlink(tree, fname2);
>+	talloc_free(mem_ctx);
>+	return ret;
>+}
>+
> struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> {
> 	struct torture_suite *suite =
>@@ -4480,6 +4532,8 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> 	torture_suite_add_1smb2_test(suite, "timeout-disconnect", test_lease_timeout_disconnect);
> 	torture_suite_add_1smb2_test(suite, "rename_wait",
> 				test_lease_rename_wait);
>+	torture_suite_add_1smb2_test(suite, "duplicate_create",
>+				test_lease_duplicate_create);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
> 
>-- 
>2.32.0
>
>
>From 7a313366d2ac0d4cfd1e912b1512f0fb716ea0b6 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Thu, 17 Feb 2022 10:58:32 -0800
>Subject: [PATCH 2/3] s4: torture: Add new SMB2 lease test
> test_lease_duplicate_open().
>
>Checks we return INVALID_PARAMETER when trying to open a
>different file with a duplicate lease key on the same share.
>
>Checked against Windows10. Currently fails against smbd
>so add knownfail.d/smb2-lease-duplicateopen
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: David Mulder <dmulder@suse.com>
>(cherry picked from commit ca3896b6f8bbcad68f042720feceedfa29ddbd83)
>---
> selftest/knownfail.d/smb2-lease-duplicateopen |  1 +
> source4/torture/smb2/lease.c                  | 70 +++++++++++++++++++
> 2 files changed, 71 insertions(+)
> create mode 100644 selftest/knownfail.d/smb2-lease-duplicateopen
>
>diff --git a/selftest/knownfail.d/smb2-lease-duplicateopen b/selftest/knownfail.d/smb2-lease-duplicateopen
>new file mode 100644
>index 00000000000..1336b02d74c
>--- /dev/null
>+++ b/selftest/knownfail.d/smb2-lease-duplicateopen
>@@ -0,0 +1 @@
>+^samba3.smb2.lease.duplicate_open\(nt4_dc\)
>diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
>index 30101c96de5..43b418c5acf 100644
>--- a/source4/torture/smb2/lease.c
>+++ b/source4/torture/smb2/lease.c
>@@ -4488,6 +4488,74 @@ done:
> 	return ret;
> }
> 
>+static bool test_lease_duplicate_open(struct torture_context *tctx,
>+				   struct smb2_tree *tree)
>+{
>+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
>+	struct smb2_create io;
>+	struct smb2_lease ls;
>+	struct smb2_handle h1 = {{0}};
>+	struct smb2_handle h2 = {{0}};
>+	NTSTATUS status;
>+	const char *fname1 = "duplicate_open1.dat";
>+	const char *fname2 = "duplicate_open2.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");
>+	}
>+
>+	/* Ensure files don't exist. */
>+	smb2_util_unlink(tree, fname1);
>+	smb2_util_unlink(tree, fname2);
>+
>+	/* Create file1 - LEASE1 key. */
>+	smb2_lease_create(&io, &ls, false, fname1, LEASE1,
>+			  smb2_util_lease_state("RWH"));
>+	status = smb2_create(tree, mem_ctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h1 = io.out.file.handle;
>+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
>+
>+	/* Leave file1 open and leased. */
>+
>+	/* Create file2 - no lease. */
>+	smb2_lease_create(&io, NULL, false, fname2, 0,
>+			  smb2_util_lease_state("RWH"));
>+	status = smb2_create(tree, mem_ctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	h2 = io.out.file.handle;
>+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
>+	/* Close it. */
>+	smb2_util_close(tree, h2);
>+
>+	/*
>+	 * Try and open file2 with the same LEASE1 key - this should fail with.
>+	 * INVALID_PARAMETER.
>+	 */
>+	smb2_lease_create(&io, &ls, false, fname2, LEASE1,
>+			  smb2_util_lease_state("RWH"));
>+	status = smb2_create(tree, mem_ctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER);
>+	/*
>+	 * If we did open this is an error, but save off
>+	 * the handle so we close below.
>+	 */
>+	h2 = io.out.file.handle;
>+
>+done:
>+	smb2_util_close(tree, h2);
>+	smb2_util_close(tree, h1);
>+	smb2_util_unlink(tree, fname1);
>+	smb2_util_unlink(tree, fname2);
>+	talloc_free(mem_ctx);
>+	return ret;
>+}
>+
> struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> {
> 	struct torture_suite *suite =
>@@ -4534,6 +4602,8 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
> 				test_lease_rename_wait);
> 	torture_suite_add_1smb2_test(suite, "duplicate_create",
> 				test_lease_duplicate_create);
>+	torture_suite_add_1smb2_test(suite, "duplicate_open",
>+				test_lease_duplicate_open);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
> 
>-- 
>2.32.0
>
>
>From 5c38b1a2935f3e6bd361702ae8d355d65b9ebb4d Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Thu, 17 Feb 2022 11:12:39 -0800
>Subject: [PATCH 3/3] s3: smbd: Fix our leases code to return the correct error
> in the non-dynamic share case.
>
>We now return INVALID_PARAMETER when trying to open a
>different file with a duplicate lease key on the same
>(non-dynamic) share. This will enable us to pass another
>Windows test suite leases test.
>
>We now behave the same as Windows10.
>
>Remove knownfail.d/smb2-lease-duplicateopen
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: David Mulder <dmulder@suse.com>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Fri Feb 18 20:12:12 UTC 2022 on sn-devel-184
>
>(cherry picked from commit 408be54323861c24b6377b804be4428cf45b471e)
>---
> selftest/knownfail.d/smb2-lease-duplicateopen |  1 -
> source3/smbd/open.c                           | 38 ++++++++++++++++++-
> 2 files changed, 36 insertions(+), 3 deletions(-)
> delete mode 100644 selftest/knownfail.d/smb2-lease-duplicateopen
>
>diff --git a/selftest/knownfail.d/smb2-lease-duplicateopen b/selftest/knownfail.d/smb2-lease-duplicateopen
>deleted file mode 100644
>index 1336b02d74c..00000000000
>--- a/selftest/knownfail.d/smb2-lease-duplicateopen
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smb2.lease.duplicate_open\(nt4_dc\)
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index a5664b319ad..5a3ac2c064a 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -5302,8 +5302,42 @@ static void lease_match_parser(
> 
> 		/* Everything should be the same. */
> 		if (!file_id_equal(&state->id, &f->id)) {
>-			/* This should catch all dynamic share cases. */
>-			state->match_status = NT_STATUS_OPLOCK_NOT_GRANTED;
>+			/*
>+			 * The client asked for a lease on a
>+			 * file that doesn't match the file_id
>+			 * in the database.
>+			 *
>+			 * Maybe this is a dynamic share, i.e.
>+			 * a share where the servicepath is
>+			 * different for different users (e.g.
>+			 * the [HOMES] share.
>+			 *
>+			 * If the servicepath is different, but the requested
>+			 * file name + stream name is the same then this is
>+			 * a dynamic share, the client is using the same share
>+			 * name and doesn't know that the underlying servicepath
>+			 * is different. It was expecting a lease on the
>+			 * same file. Return NT_STATUS_OPLOCK_NOT_GRANTED
>+			 * to break leases
>+			 *
>+			 * Otherwise the client has messed up, or is
>+			 * testing our error codes, so return
>+			 * NT_STATUS_INVALID_PARAMETER.
>+			 */
>+			if (!strequal(f->servicepath, state->servicepath) &&
>+			    strequal(f->base_name, state->fname->base_name) &&
>+			    strequal(f->stream_name, state->fname->stream_name))
>+			{
>+				/*
>+				 * Name is the same but servicepath is
>+				 * different, dynamic share. Break leases.
>+				 */
>+				state->match_status =
>+					NT_STATUS_OPLOCK_NOT_GRANTED;
>+			} else {
>+				state->match_status =
>+					NT_STATUS_INVALID_PARAMETER;
>+			}
> 			break;
> 		}
> 		if (!strequal(f->servicepath, state->servicepath)) {
>-- 
>2.32.0
>

Flags:

jra: review? (dmulder)
npower: review+

Actions: View

Attachments on bug 14737: 16653 | 17169 | 17170