The Samba-Bugzilla – Attachment 18478 Details for
Bug 15732
smbd fails to correctly check sharemode against OVERWRITE dispositions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.20 and 4.21 cherry-picked from master
bug15732-420,421.patch (text/plain), 17.81 KB, created by
Ralph Böhme
on 2024-10-17 09:03:07 UTC
(
hide
)
Description:
Patch for 4.20 and 4.21 cherry-picked from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2024-10-17 09:03:07 UTC
Size:
17.81 KB
patch
obsolete
>From e16b37187252c6bc0a1c997c02fa6f01a5ccee3d Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 2 Oct 2024 14:07:49 +0200 >Subject: [PATCH 1/4] smbtorture: prepare test_overwrite_read_only_file() for > more subtests > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit f88e52a6f487a216dbb805fabc08e862abb9b643) >--- > source4/torture/smb2/acls.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > >diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c >index 019886ebcd21..f9c1158ee47c 100644 >--- a/source4/torture/smb2/acls.c >+++ b/source4/torture/smb2/acls.c >@@ -2993,7 +2993,7 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > struct smb2_tree *tree) > { > NTSTATUS status; >- struct smb2_create c; >+ struct smb2_create c = {}; > const char *fname = BASEDIR "\\test_overwrite_read_only_file.txt"; > struct smb2_handle handle = {{0}}; > union smb_fileinfo q; >@@ -3007,12 +3007,15 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > int disposition; > const char *disposition_string; > NTSTATUS expected_status; >- } tcases[] = { >+ }; >+ > #define TCASE(d, s) { \ > .disposition = d, \ > .disposition_string = #d, \ > .expected_status = s, \ > } >+ >+ struct tcase fs_tcases[] = { > TCASE(NTCREATEX_DISP_OPEN, NT_STATUS_OK), > TCASE(NTCREATEX_DISP_SUPERSEDE, NT_STATUS_ACCESS_DENIED), > TCASE(NTCREATEX_DISP_OVERWRITE, NT_STATUS_ACCESS_DENIED), >@@ -3075,12 +3078,12 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > smb2_util_close(tree, handle); > ZERO_STRUCT(handle); > >- for (i = 0; i < ARRAY_SIZE(tcases); i++) { >+ for (i = 0; i < ARRAY_SIZE(fs_tcases); i++) { > torture_comment(tctx, "Verify open with %s disposition\n", >- tcases[i].disposition_string); >+ fs_tcases[i].disposition_string); > > c = (struct smb2_create) { >- .in.create_disposition = tcases[i].disposition, >+ .in.create_disposition = fs_tcases[i].disposition, > .in.desired_access = SEC_FILE_READ_DATA, > .in.file_attributes = FILE_ATTRIBUTE_NORMAL, > .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, >@@ -3091,7 +3094,7 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > status = smb2_create(tree, tctx, &c); > smb2_util_close(tree, c.out.file.handle); > torture_assert_ntstatus_equal_goto( >- tctx, status, tcases[i].expected_status, ret, done, >+ tctx, status, fs_tcases[i].expected_status, ret, done, > "smb2_create failed\n"); > }; > >-- >2.47.0 > > >From 7cd780d33bc043bb42202c71f661c69e236ad08c Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 2 Oct 2024 18:17:17 +0200 >Subject: [PATCH 2/4] smbtorture: fix smb2.notify.mask test > >The strange function custom_smb2_create() was somehow causing >NT_STATUS_DELETE_PENDING failures: > > failure: mask [ > (../../source4/torture/smb2/notify.c:490) Incorrect status NT_STATUS_DELETE_PENDING - should be NT_STATUS_OK > ] > >I couldn't figure out what was causing this exactly, but after doing these >cleanups the error went away. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit 4591f27ca81dff997ef7474565fc9c373abfa4a9) >--- > source4/torture/smb2/notify.c | 34 ++++++++++++++-------------------- > 1 file changed, 14 insertions(+), 20 deletions(-) > >diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c >index 0aadc50c6077..b76cb7967aa2 100644 >--- a/source4/torture/smb2/notify.c >+++ b/source4/torture/smb2/notify.c >@@ -944,6 +944,7 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > notify.smb2.out.changes[0].action, \ > notify.smb2.in.completion_filter); \ > ret = false; \ >+ goto done; \ > } else if (notify.smb2.out.changes[0].action != Action) { \ > torture_result(torture, TORTURE_FAIL, \ > "ERROR: nchanges=%d action=%d " \ >@@ -953,6 +954,7 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > Action, \ > notify.smb2.in.completion_filter); \ > ret = false; \ >+ goto done; \ > } else if (strcmp(notify.smb2.out.changes[0].name.s, \ > "tname1") != 0) { \ > torture_result(torture, TORTURE_FAIL, \ >@@ -963,6 +965,7 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > notify.smb2.in.completion_filter, \ > notify.smb2.out.changes[0].name.s); \ > ret = false; \ >+ goto done; \ > } \ > } \ > } while (0); \ >@@ -1016,14 +1019,12 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > torture_comment(torture, "Testing rename file\n"); > ZERO_STRUCT(sinfo); > sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; >- sinfo.rename_information.in.file.handle = h1; > sinfo.rename_information.in.overwrite = true; > sinfo.rename_information.in.root_fid = 0; > sinfo.rename_information.in.new_name = BASEDIR_MSK "\\tname2"; > NOTIFY_MASK_TEST("Testing rename file", >- smb2_util_close(tree2, custom_smb2_create(tree2, >- torture, &(io1.smb2)));, >- smb2_setinfo_file(tree2, &sinfo);, >+ torture_smb2_testfile(tree2, BASEDIR_MSK "\\tname1", &h2);, >+ (sinfo.rename_information.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname2");, > NOTIFY_ACTION_OLD_NAME, > FILE_NOTIFY_CHANGE_FILE_NAME, 2); >@@ -1031,21 +1032,19 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > torture_comment(torture, "Testing rename dir\n"); > ZERO_STRUCT(sinfo); > sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; >- sinfo.rename_information.in.file.handle = h1; > sinfo.rename_information.in.overwrite = true; > sinfo.rename_information.in.root_fid = 0; > sinfo.rename_information.in.new_name = BASEDIR_MSK "\\tname2"; > NOTIFY_MASK_TEST("Testing rename dir", >- smb2_util_mkdir(tree2, BASEDIR_MSK "\\tname1");, >- smb2_setinfo_file(tree2, &sinfo);, >- smb2_util_rmdir(tree2, BASEDIR_MSK "\\tname2");, >+ torture_smb2_testdir(tree2, BASEDIR_MSK "\\tname1", &h2);, >+ (sinfo.rename_information.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, >+ (smb2_util_close(tree2, h2), smb2_util_rmdir(tree2, BASEDIR_MSK "\\tname2"));, > NOTIFY_ACTION_OLD_NAME, > FILE_NOTIFY_CHANGE_DIR_NAME, 2); > > torture_comment(torture, "Testing set path attribute\n"); > NOTIFY_MASK_TEST("Testing set path attribute", >- smb2_util_close(tree2, custom_smb2_create(tree2, >- torture, &(io.smb2)));, >+ torture_setup_simple_file(torture, tree2, BASEDIR_MSK "\\tname1");, > smb2_util_setatr(tree2, BASEDIR_MSK "\\tname1", > FILE_ATTRIBUTE_HIDDEN);, > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname1");, >@@ -1055,12 +1054,10 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > torture_comment(torture, "Testing set path write time\n"); > ZERO_STRUCT(sinfo); > sinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; >- sinfo.generic.in.file.handle = h1; > sinfo.basic_info.in.write_time = 1000; > NOTIFY_MASK_TEST("Testing set path write time", >- smb2_util_close(tree2, custom_smb2_create(tree2, >- torture, &(io1.smb2)));, >- smb2_setinfo_file(tree2, &sinfo);, >+ torture_setup_simple_file(torture, tree2, BASEDIR_MSK "\\tname1");, >+ (sinfo.generic.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname1");, > NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_LAST_WRITE, 1); >@@ -1073,13 +1070,12 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > else { > ZERO_STRUCT(sinfo); > sinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; >- sinfo.generic.in.file.handle = h1; > sinfo.basic_info.in.create_time = 0; > torture_comment(torture, "Testing set file create time\n"); > NOTIFY_MASK_TEST("Testing set file create time", > smb2_create_complex_file(torture, tree2, > BASEDIR_MSK "\\tname1", &h2);, >- smb2_setinfo_file(tree2, &sinfo);, >+ (sinfo.generic.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, > (smb2_util_close(tree2, h2), > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname1"));, > NOTIFY_ACTION_MODIFIED, >@@ -1088,7 +1084,6 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > > ZERO_STRUCT(sinfo); > sinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; >- sinfo.generic.in.file.handle = h1; > sinfo.basic_info.in.access_time = 0; > torture_comment(torture, "Testing set file access time\n"); > NOTIFY_MASK_TEST("Testing set file access time", >@@ -1096,7 +1091,7 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > tree2, > BASEDIR_MSK "\\tname1", > &h2);, >- smb2_setinfo_file(tree2, &sinfo);, >+ (sinfo.generic.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, > (smb2_util_close(tree2, h2), > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname1"));, > NOTIFY_ACTION_MODIFIED, >@@ -1104,7 +1099,6 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > > ZERO_STRUCT(sinfo); > sinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; >- sinfo.generic.in.file.handle = h1; > sinfo.basic_info.in.change_time = 0; > torture_comment(torture, "Testing set file change time\n"); > NOTIFY_MASK_TEST("Testing set file change time", >@@ -1112,7 +1106,7 @@ static bool torture_smb2_notify_mask(struct torture_context *torture, > tree2, > BASEDIR_MSK "\\tname1", > &h2);, >- smb2_setinfo_file(tree2, &sinfo);, >+ (sinfo.generic.in.file.handle = h2, smb2_setinfo_file(tree2, &sinfo));, > (smb2_util_close(tree2, h2), > smb2_util_unlink(tree2, BASEDIR_MSK "\\tname1"));, > NOTIFY_ACTION_MODIFIED, >-- >2.47.0 > > >From 0f4887eccd4a009daae7286100ebdbee806ac910 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 2 Oct 2024 14:08:36 +0200 >Subject: [PATCH 3/4] smbtorture: add subtests for overwrite dispositions vs > sharemodes > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit 849afe05ade140898b1eab9b28d46edc8357c844) >--- > selftest/knownfail.d/samba3.smb2.acls | 1 + > source4/torture/smb2/acls.c | 109 +++++++++++++++++++++++++- > 2 files changed, 108 insertions(+), 2 deletions(-) > create mode 100644 selftest/knownfail.d/samba3.smb2.acls > >diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls >new file mode 100644 >index 000000000000..18df260c0e50 >--- /dev/null >+++ b/selftest/knownfail.d/samba3.smb2.acls >@@ -0,0 +1 @@ >+^samba3.smb2.acls.OVERWRITE_READ_ONLY_FILE >diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c >index f9c1158ee47c..0459d6547dc7 100644 >--- a/source4/torture/smb2/acls.c >+++ b/source4/torture/smb2/acls.c >@@ -2994,8 +2994,10 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > { > NTSTATUS status; > struct smb2_create c = {}; >+ struct smb2_create c2 = {}; > const char *fname = BASEDIR "\\test_overwrite_read_only_file.txt"; > struct smb2_handle handle = {{0}}; >+ struct smb2_handle h2 = {}; > union smb_fileinfo q; > union smb_setfileinfo set; > struct security_descriptor *sd = NULL, *sd_orig = NULL; >@@ -3021,6 +3023,12 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > TCASE(NTCREATEX_DISP_OVERWRITE, NT_STATUS_ACCESS_DENIED), > TCASE(NTCREATEX_DISP_OVERWRITE_IF, NT_STATUS_ACCESS_DENIED), > }; >+ >+ struct tcase sharing_tcases[] = { >+ TCASE(NTCREATEX_DISP_SUPERSEDE, NT_STATUS_SHARING_VIOLATION), >+ TCASE(NTCREATEX_DISP_OVERWRITE, NT_STATUS_SHARING_VIOLATION), >+ TCASE(NTCREATEX_DISP_OVERWRITE_IF, NT_STATUS_SHARING_VIOLATION), >+ }; > #undef TCASE > > ret = smb2_util_setup_dir(tctx, tree, BASEDIR); >@@ -3124,11 +3132,108 @@ static bool test_overwrite_read_only_file(struct torture_context *tctx, > torture_assert_ntstatus_ok_goto(tctx, status, ret, done, > "smb2_setinfo_file failed\n"); > >- smb2_util_close(tree, handle); >+ status = smb2_util_close(tree, handle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_close failed\n"); > ZERO_STRUCT(handle); > >+ for (i = 0; i < ARRAY_SIZE(sharing_tcases); i++) { >+ struct tcase *tcase = &sharing_tcases[i]; >+ >+ torture_comment(tctx, "Verify %s disposition\n", >+ tcase->disposition_string); >+ >+ torture_comment(tctx, "Read-nonly open file with SHARE_READ\n"); >+ >+ c = (struct smb2_create) { >+ .in.desired_access = SEC_FILE_READ_DATA, >+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL, >+ .in.share_access = NTCREATEX_SHARE_ACCESS_READ, >+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF, >+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, >+ .in.fname = fname, >+ }; >+ >+ status = smb2_create(tree, tctx, &c); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_create failed\n"); >+ handle = c.out.file.handle; >+ >+ torture_comment(tctx, "A second open with %s must return %s\n", >+ tcase->disposition_string, nt_errstr(tcase->expected_status)); >+ >+ c2 = (struct smb2_create) { >+ .in.desired_access = SEC_FILE_READ_DATA, >+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL, >+ .in.share_access = NTCREATEX_SHARE_ACCESS_READ, >+ .in.create_disposition = tcase->disposition, >+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, >+ .in.fname = fname, >+ }; >+ >+ status = smb2_create(tree, tctx, &c2); >+ torture_assert_ntstatus_equal_goto(tctx, status, >+ tcase->expected_status, >+ ret, done, >+ "Wrong status code\n"); >+ >+ status = smb2_util_close(tree, handle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_close failed\n"); >+ ZERO_STRUCT(handle); >+ >+ torture_comment(tctx, "First open with %s\n", >+ tcase->disposition_string); >+ >+ c = (struct smb2_create) { >+ .in.desired_access = SEC_FILE_READ_DATA, >+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL, >+ .in.share_access = NTCREATEX_SHARE_ACCESS_READ, >+ .in.create_disposition = tcase->disposition, >+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, >+ .in.fname = fname, >+ }; >+ >+ status = smb2_create(tree, tctx, &c); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_create failed\n"); >+ handle = c.out.file.handle; >+ >+ torture_comment(tctx, "A second read-only open with SHARE_READ " >+ "must work\n"); >+ >+ c = (struct smb2_create) { >+ .in.desired_access = SEC_FILE_READ_DATA, >+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL, >+ .in.share_access = NTCREATEX_SHARE_ACCESS_READ, >+ .in.create_disposition = NTCREATEX_DISP_OPEN, >+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, >+ .in.fname = fname, >+ }; >+ >+ status = smb2_create(tree, tctx, &c); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_create failed\n"); >+ h2 = c.out.file.handle; >+ >+ status = smb2_util_close(tree, handle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_close failed\n"); >+ ZERO_STRUCT(handle); >+ >+ status = smb2_util_close(tree, h2); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_close failed\n"); >+ ZERO_STRUCT(h2); >+ } >+ > done: >- smb2_util_close(tree, handle); >+ if (!smb2_util_handle_empty(handle)) { >+ smb2_util_close(tree, handle); >+ } >+ if (!smb2_util_handle_empty(h2)) { >+ smb2_util_close(tree, h2); >+ } > smb2_util_unlink(tree, fname); > smb2_deltree(tree, BASEDIR); > return ret; >-- >2.47.0 > > >From 331ec0992661bc79dca5de0a186f49448b686c67 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 2 Oct 2024 14:09:33 +0200 >Subject: [PATCH 4/4] smbd: fix share access check for overwrite dispostions >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15732 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> > >Autobuild-User(master): Ralph Böhme <slow@samba.org> >Autobuild-Date(master): Mon Oct 14 12:23:04 UTC 2024 on atb-devel-224 > >(cherry picked from commit 6140c3177a0330f42411618c3fca28930ea02a21) >--- > selftest/knownfail.d/samba3.smb2.acls | 1 - > source3/smbd/open.c | 6 +++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > delete mode 100644 selftest/knownfail.d/samba3.smb2.acls > >diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls >deleted file mode 100644 >index 18df260c0e50..000000000000 >--- a/selftest/knownfail.d/samba3.smb2.acls >+++ /dev/null >@@ -1 +0,0 @@ >-^samba3.smb2.acls.OVERWRITE_READ_ONLY_FILE >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 22a39ac35ef6..ef3f65fea0fc 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -3389,6 +3389,7 @@ static NTSTATUS check_and_store_share_mode( > struct share_mode_lock *lck, > uint32_t create_disposition, > uint32_t access_mask, >+ uint32_t open_access_mask, > uint32_t share_access, > int oplock_request, > const struct smb2_lease *lease, >@@ -3415,7 +3416,7 @@ static NTSTATUS check_and_store_share_mode( > status = handle_share_mode_lease(fsp, > lck, > create_disposition, >- access_mask, >+ open_access_mask, > share_access, > oplock_request, > lease, >@@ -3745,6 +3746,7 @@ struct open_ntcreate_lock_state { > struct smb_request *req; > uint32_t create_disposition; > uint32_t access_mask; >+ uint32_t open_access_mask; > uint32_t share_access; > int oplock_request; > const struct smb2_lease *lease; >@@ -3773,6 +3775,7 @@ static void open_ntcreate_lock_add_entry(struct share_mode_lock *lck, > lck, > state->create_disposition, > state->access_mask, >+ state->open_access_mask, > state->share_access, > state->oplock_request, > state->lease, >@@ -4407,6 +4410,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > .req = req, > .create_disposition = create_disposition, > .access_mask = access_mask, >+ .open_access_mask = open_access_mask, > .share_access = share_access, > .oplock_request = oplock_request, > .lease = lease, >-- >2.47.0 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18478">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 15732
:
18478
|
18489