From 62b79f2d15557e1bb09dcc8d8806702167d9c255 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 24 Jan 2022 16:54:29 +0100 Subject: [PATCH 1/7] CI: remove shares referencing removed functionality The whole "smbd:force sync [user|root] [path|chdir] safe threadpool" stuff was removed long ago by 29dd6f3e59055a17fa3d6a63619773f940e63374. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14957 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 1e3e22cc45583cb11ef5dbc3c044bf6189fe6036) --- selftest/target/Samba3.pm | 52 --------------------------------------- source3/selftest/tests.py | 4 +-- 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index c1d0c60d96a..4c7833b422b 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1539,32 +1539,6 @@ sub setup_simpleserver aio_pthread:aio open = yes smbd async dosmode = yes -[vfs_aio_pthread_async_dosmode_force_sync1] - path = $prefix_abs/share - read only = no - vfs objects = aio_pthread - store dos attributes = yes - aio_pthread:aio open = yes - smbd async dosmode = yes - # This simulates non linux systems - smbd:force sync user path safe threadpool = yes - smbd:force sync user chdir safe threadpool = yes - smbd:force sync root path safe threadpool = yes - smbd:force sync root chdir safe threadpool = yes - -[vfs_aio_pthread_async_dosmode_force_sync2] - path = $prefix_abs/share - read only = no - vfs objects = aio_pthread xattr_tdb - store dos attributes = yes - aio_pthread:aio open = yes - smbd async dosmode = yes - # This simulates non linux systems - smbd:force sync user path safe threadpool = yes - smbd:force sync user chdir safe threadpool = yes - smbd:force sync root path safe threadpool = yes - smbd:force sync root chdir safe threadpool = yes - [vfs_aio_fork] path = $prefix_abs/share vfs objects = aio_fork @@ -1940,32 +1914,6 @@ sub setup_fileserver_smb1 aio_pthread:aio open = yes smbd async dosmode = yes -[vfs_aio_pthread_async_dosmode_force_sync1] - path = $prefix_abs/share - read only = no - vfs objects = aio_pthread - store dos attributes = yes - aio_pthread:aio open = yes - smbd async dosmode = yes - # This simulates non linux systems - smbd:force sync user path safe threadpool = yes - smbd:force sync user chdir safe threadpool = yes - smbd:force sync root path safe threadpool = yes - smbd:force sync root chdir safe threadpool = yes - -[vfs_aio_pthread_async_dosmode_force_sync2] - path = $prefix_abs/share - read only = no - vfs objects = aio_pthread xattr_tdb - store dos attributes = yes - aio_pthread:aio open = yes - smbd async dosmode = yes - # This simulates non linux systems - smbd:force sync user path safe threadpool = yes - smbd:force sync user chdir safe threadpool = yes - smbd:force sync root path safe threadpool = yes - smbd:force sync root chdir safe threadpool = yes - [vfs_aio_fork] path = $prefix_abs/share vfs objects = aio_fork diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 6b146c76381..185608ad27c 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -275,9 +275,7 @@ plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONEMPTY", shares = [ "vfs_aio_pthread_async_dosmode_default1", - "vfs_aio_pthread_async_dosmode_default2", - "vfs_aio_pthread_async_dosmode_force_sync1", - "vfs_aio_pthread_async_dosmode_force_sync2" + "vfs_aio_pthread_async_dosmode_default2" ] for s in shares: plantestsuite("samba3.smbtorture_s3.%s(simpleserver).SMB2-BASIC" % s, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), 'SMB2-BASIC', '//$SERVER_IP/' + s, '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) -- 2.32.0 From cb5899a178bd02bb30ffaa166e5791e86a800fa2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 24 Jan 2022 16:45:11 +0100 Subject: [PATCH 2/7] smbd: check "store dos attributes" settings in the async dosmode code BUG: https://bugzilla.samba.org/show_bug.cgi?id=14957 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit ecf56c1d9b6b898ed4060e3bba341392ddcc9b5a) --- source3/modules/vfs_default.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 5701e37d5ec..26595d36848 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1785,6 +1785,14 @@ static struct tevent_req *vfswrap_get_dos_attributes_send( .smb_fname = smb_fname, }; + if (!lp_store_dos_attributes(SNUM(dir_fsp->conn))) { + DBG_ERR("%s: \"smbd async dosmode\" enabled, but " + "\"store dos attributes\" is disabled\n", + dir_fsp->conn->connectpath); + tevent_req_nterror(req, NT_STATUS_NOT_IMPLEMENTED); + return tevent_req_post(req, ev); + } + subreq = SMB_VFS_GETXATTRAT_SEND(state, ev, dir_fsp, -- 2.32.0 From d7bdc0e5e32d06c212e2cdb351452d6af604ac3b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 25 Jan 2022 17:59:37 +0100 Subject: [PATCH 3/7] CI: add test "smb2.async_dosmode" Verifies async-dosmode sync fallback works with shadow_copy2 which returns ENOSYS for SMB_VFS_GET_DOS_ATTRIBUTES_SEND(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14957 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit ffdb1c3e00c233efc99e8f1a66a5f83beb4e07f3) --- .../samba3.smb2.async_dosmode.async_dosmode | 1 + selftest/target/Samba3.pm | 6 ++ source3/selftest/tests.py | 4 ++ source4/selftest/tests.py | 1 + source4/torture/smb2/dosmode.c | 71 +++++++++++++++++++ source4/torture/smb2/smb2.c | 1 + 6 files changed, 84 insertions(+) create mode 100644 selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode diff --git a/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode b/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode new file mode 100644 index 00000000000..a28337150c8 --- /dev/null +++ b/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode @@ -0,0 +1 @@ +^samba3.smb2.async_dosmode.async_dosmode\(simpleserver\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 4c7833b422b..116981f044a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1539,6 +1539,12 @@ sub setup_simpleserver aio_pthread:aio open = yes smbd async dosmode = yes +[async_dosmode_shadow_copy2] + path = $prefix_abs/share + read only = no + vfs objects = shadow_copy2 xattr_tdb + smbd async dosmode = yes + [vfs_aio_fork] path = $prefix_abs/share vfs objects = aio_fork diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 185608ad27c..4337de1d1a8 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -975,6 +975,10 @@ for t in tests: plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') elif t == "smb2.acls_non_canonical": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/acls_non_canonical -U$USERNAME%$PASSWORD') + elif t == "smb2.async_dosmode": + plansmbtorture4testsuite("smb2.async_dosmode", + "simpleserver", + "//$SERVER_IP/async_dosmode_shadow_copy2 -U$USERNAME%$PASSWORD") elif t == "rpc.wkssvc": plansmbtorture4testsuite(t, "ad_member", '//$SERVER/tmp -U$DC_USERNAME%$DC_PASSWORD') elif t == "rpc.srvsvc": diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 3a6a716f061..a9d00755109 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -371,6 +371,7 @@ smb2_s3only = [ "smb2.fileid", "smb2.fileid_unique", "smb2.timestamps", + "smb2.async_dosmode", ] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] diff --git a/source4/torture/smb2/dosmode.c b/source4/torture/smb2/dosmode.c index 7808ca67dba..7610a20329f 100644 --- a/source4/torture/smb2/dosmode.c +++ b/source4/torture/smb2/dosmode.c @@ -181,3 +181,74 @@ done: smb2_deltree(tree, dname); return ret; } + +bool torture_smb2_async_dosmode(struct torture_context *tctx) +{ + bool ret = true; + NTSTATUS status; + struct smb2_tree *tree = NULL; + const char *dname = "torture_dosmode"; + const char *fname = "torture_dosmode\\file"; + struct smb2_handle h = {{0}}; + struct smb2_create io; + union smb_setfileinfo sfinfo; + struct smb2_find f; + union smb_search_data *d; + unsigned int count; + + if (!torture_smb2_connection(tctx, &tree)) { + return false; + } + + smb2_deltree(tree, dname); + + status = torture_smb2_testdir(tree, dname, &h); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir failed"); + + ZERO_STRUCT(io); + io.in.desired_access = SEC_FLAG_MAXIMUM_ALLOWED; + io.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + io.in.create_disposition = NTCREATEX_DISP_CREATE; + io.in.create_options = 0; + io.in.fname = fname; + + status = smb2_create(tree, tctx, &io); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed"); + + ZERO_STRUCT(sfinfo); + sfinfo.basic_info.in.attrib = FILE_ATTRIBUTE_HIDDEN; + sfinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; + sfinfo.generic.in.file.handle = io.out.file.handle; + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_setinfo_filefailed"); + + smb2_util_close(tree, io.out.file.handle); + + ZERO_STRUCT(f); + f.in.file.handle = h; + f.in.pattern = "file"; + f.in.continue_flags = SMB2_CONTINUE_FLAG_RESTART; + f.in.max_response_size = 0x1000; + f.in.level = SMB2_FIND_BOTH_DIRECTORY_INFO; + + status = smb2_find_level(tree, tree, &f, &count, &d); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, ""); + + smb2_util_close(tree, h); + ZERO_STRUCT(h); + + torture_assert_goto(tctx, + d->both_directory_info.attrib & FILE_ATTRIBUTE_HIDDEN, + ret, done, + "FILE_ATTRIBUTE_HIDDEN is not set\n"); + +done: + if (!smb2_util_handle_empty(h)) { + smb2_util_close(tree, h); + } + smb2_deltree(tree, dname); + return ret; +} diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index 95a7b49952f..3c69d8c7fa0 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -188,6 +188,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) torture_suite_add_suite(suite, torture_smb2_session_init(suite)); torture_suite_add_suite(suite, torture_smb2_replay_init(suite)); torture_suite_add_simple_test(suite, "dosmode", torture_smb2_dosmode); + torture_suite_add_simple_test(suite, "async_dosmode", torture_smb2_async_dosmode); torture_suite_add_simple_test(suite, "maxfid", torture_smb2_maxfid); torture_suite_add_simple_test(suite, "hold-sharemode", torture_smb2_hold_sharemode); -- 2.32.0 From 095c3fcb2d3b54b610cd429b386d6220f7dc37bb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 17 Dec 2021 15:02:06 +0100 Subject: [PATCH 4/7] smbd: also check for NT_STATUS_NOT_SUPPORTED If a VFS module fails SMB_VFS_GETXATTRAT_SEND/RECV with ENOSYS like currently vfs_shadow_copy2 or any other module that uses vfs_not_implemented_getxattrat_send() the ENOSYS error that vfs_not_implemented_getxattrat_send() sets gets mapped to NT_STATUS_NOT_SUPPORTED by map_nt_error_from_unix(). Unfortunately when checking whether the async SMB_VFS_GETXATTRAT_SEND() failed and to determine if the sync fallback should be triggered, we currently only check for NT_STATUS_NOT_IMPLEMENTED which is the error we get when "store dos attributes" is disabled. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14957 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 97caec07ffd18f247134d21c3ba07c31591863bc) --- .../knownfail.d/samba3.smb2.async_dosmode.async_dosmode | 1 - source3/smbd/dosmode.c | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode diff --git a/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode b/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode deleted file mode 100644 index a28337150c8..00000000000 --- a/selftest/knownfail.d/samba3.smb2.async_dosmode.async_dosmode +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.async_dosmode.async_dosmode\(simpleserver\) diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index 58fb0535c11..078ce6df07c 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -854,10 +854,13 @@ static void dos_mode_at_vfs_get_dosmode_done(struct tevent_req *subreq) * dos_mode_post() which also does the mapping of a last ressort * from S_IFMT(st_mode). * - * Only if we get NT_STATUS_NOT_IMPLEMENTED from a stacked VFS - * module we must fallback to sync processing. + * Only if we get NT_STATUS_NOT_IMPLEMENTED or + * NT_STATUS_NOT_SUPPORTED from a stacked VFS module we must + * fallback to sync processing. */ - if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) { + if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED) && + !NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED)) + { /* * state->dosmode should still be 0, but reset * it to be sure. -- 2.32.0 From a386f4645f3d32ecc6ca8da2ddff3a89d4aace7d Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 23 Feb 2022 18:14:38 +0100 Subject: [PATCH 5/7] CI: enable "smbd async dosmode" on shadow_write share Existing tests don't care, upcoming new test needs it. Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 48f81b4e7216e4dad0a86aca75890c32117a342e) --- selftest/target/Samba3.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 116981f044a..0d95d43cbcf 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -3101,6 +3101,7 @@ sub provision($$) error_inject:pwrite = EBADF shadow:mountpoint = $shadow_tstdir shadow:fixinodes = yes + smbd async dosmode = yes [dfq] path = $shrdir/dfree -- 2.32.0 From ea29f0dab13075ebde14c33ee6c57ea7cdcbf888 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 23 Feb 2022 18:10:59 +0100 Subject: [PATCH 6/7] CI: add a test for async dosmode on a file in a shadow_copy2 snapshot Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 1097b1d0776661d873861672ca38e5892014725d) --- .../script/tests/test_shadow_copy_torture.sh | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/source3/script/tests/test_shadow_copy_torture.sh b/source3/script/tests/test_shadow_copy_torture.sh index e5dc0192e95..4ae2f9f707f 100755 --- a/source3/script/tests/test_shadow_copy_torture.sh +++ b/source3/script/tests/test_shadow_copy_torture.sh @@ -150,6 +150,47 @@ test_shadow_copy_fix_inodes() echo $out | grep "hardlink: for read/write fnum 1" || return 1 } +build_hiddenfile() +{ + local snapdir + + snapdir=$WORKDIR/.snapshots + + #delete snapshots from previous tests + find $WORKDIR -name ".snapshots" -exec rm -rf {} \; 1>/dev/null 2>&1 + build_snapshots + + touch $WORKDIR/hiddenfile + + # Create a file with hidden attribute + $SMBCLIENT -U $USERNAME%$PASSWORD \ + "//$SERVER/shadow_write" \ + -c "put $WORKDIR/hiddenfile hiddenfile; setmode hiddenfile +h" + # ...and move it to the snapshot directory + mv $WORKDIR/hiddenfile $snapdir/$SNAPSHOT/ +} + +test_hiddenfile() +{ + build_hiddenfile + + out=$($SMBCLIENT \ + -U $USERNAME%$PASSWORD \ + "//$SERVER/shadow_write" \ + -c "allinfo $SNAPSHOT/hiddenfile") || return 1 + echo $out + echo $out | grep "attributes: HA (22)" || return 1 + + out=$($SMBCLIENT \ + -U $USERNAME%$PASSWORD \ + "//$SERVER/shadow_write" \ + -c "ls $SNAPSHOT/hiddenfile") || return 1 + echo $out + echo $out | grep "hiddenfile[[:blank:]]*AH" || return 1 + + return 0 +} + build_files $WORKDIR # test open for writing and write behaviour of snapshoted files @@ -161,4 +202,6 @@ test_shadow_copy_openroot "opening root of shadow copy share" testit "fix inodes with hardlink" test_shadow_copy_fix_inodes || failed=`expr $failed + 1` +testit "Test reading DOS attribute" test_hiddenfile || failed=`expr $failed + 1` + exit $failed -- 2.32.0 From 33058836570ccd388b78f011c24cf340388770df Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 23 Feb 2022 11:36:29 +0100 Subject: [PATCH 7/7] vfs_shadow_copy2: remove async getxattrat vfswrap_getxattrat_send() is handle based using smb_fname->fsp. As the open of smb_fname->fsp was processed by this module, the handle is already correctly opened on the file in the snapshot. In the end this means we can just call directly call the next function here. Note that the same reasoning might apply to other modules that use vfs_not_implemented_getxattrat_send(), but checking and adjusting those is a job for another day. Currently they will continue to go via the sync fallback of the caller. Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Mon Feb 28 20:53:35 UTC 2022 on sn-devel-184 (cherry picked from commit afc2103da0fe947afc027b3e25c5e82aa5d3f1fb) --- source3/modules/vfs_shadow_copy2.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index d61b3eac4b9..769f8c7da87 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -3256,8 +3256,6 @@ static struct vfs_fn_pointers vfs_shadow_copy2_fns = { .realpath_fn = shadow_copy2_realpath, .get_shadow_copy_data_fn = shadow_copy2_get_shadow_copy_data, .mkdirat_fn = shadow_copy2_mkdirat, - .getxattrat_send_fn = vfs_not_implemented_getxattrat_send, - .getxattrat_recv_fn = vfs_not_implemented_getxattrat_recv, .fsetxattr_fn = shadow_copy2_fsetxattr, .fchflags_fn = shadow_copy2_fchflags, .get_real_filename_fn = shadow_copy2_get_real_filename, -- 2.32.0