The Samba-Bugzilla – Attachment 17170 Details for
Bug 14737
Samba does not response STATUS_INVALID_PARAMETER when opening 2 objects with same lease key
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.14.next.
bug-14737-4.14 (text/plain), 9.80 KB, created by
Jeremy Allison
on 2022-02-18 20:42:08 UTC
(
hide
)
Description:
git-am fix for 4.14.next.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2022-02-18 20:42:08 UTC
Size:
9.80 KB
patch
obsolete
>From 59bd596ea59d6205637939862f58a64a83d78fb7 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 824db95a4b5..e7407453006 100644 >--- a/source4/torture/smb2/lease.c >+++ b/source4/torture/smb2/lease.c >@@ -4293,6 +4293,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 = >@@ -4336,6 +4388,8 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) > torture_suite_add_1smb2_test(suite, "unlink", test_lease_unlink); > 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 f438b1797d3f1e31825a0e831a5ce0eed6e9de92 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 e7407453006..8f2eaee469e 100644 >--- a/source4/torture/smb2/lease.c >+++ b/source4/torture/smb2/lease.c >@@ -4345,6 +4345,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 = >@@ -4390,6 +4458,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 a1a3447546a9ba9d588d38b44366ec8f051189c6 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 b0bf0dfb568..2c3bf9e68e7 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -5350,8 +5350,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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
jra
:
review?
(
dmulder
)
npower
:
review+
Actions:
View
Attachments on
bug 14737
:
16653
|
17169
| 17170