From 1be7593915fec986fb282ae3f62377221ff464ad Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 10 Nov 2022 14:41:15 -0800 Subject: [PATCH 1/2] s3: smbd: Add test to show smbd crashes when doing an FSCTL on a named stream handle. Add knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15236 Signed-off-by: Andrew Walker Reviewed-by: Jeremy Allison (cherry picked from commit abc4495e4591964bb4625c2669a1f84213faab77) --- selftest/knownfail | 1 + selftest/knownfail.d/smb2-ioctl-stream | 1 + source3/selftest/tests.py | 2 + source4/torture/smb2/ioctl.c | 74 ++++++++++++++++++++++++++ source4/torture/smb2/smb2.c | 2 + 5 files changed, 80 insertions(+) create mode 100644 selftest/knownfail.d/smb2-ioctl-stream diff --git a/selftest/knownfail b/selftest/knownfail index 82dd7e1e8b4..db8603781bb 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -202,6 +202,7 @@ ^samba4.smb2.ioctl.copy_chunk_\w*\(ad_dc_ntvfs\) # not supported by s4 ntvfs server ^samba4.smb2.ioctl.copy-chunk streams\(ad_dc_ntvfs\) # not supported by s4 ntvfs server ^samba4.smb2.ioctl.bug14769\(ad_dc_ntvfs\) # not supported by s4 ntvfs server +^samba4.smb2.ioctl-on-stream.ioctl-on-stream\(ad_dc_ntvfs\) ^samba3.smb2.dir.one ^samba3.smb2.dir.modify ^samba3.smb2.oplock.batch20 diff --git a/selftest/knownfail.d/smb2-ioctl-stream b/selftest/knownfail.d/smb2-ioctl-stream new file mode 100644 index 00000000000..518726e8f19 --- /dev/null +++ b/selftest/knownfail.d/smb2-ioctl-stream @@ -0,0 +1 @@ +^samba3.smb2.ioctl-on-stream.ioctl-on-stream\(fileserver\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 01ec90e9878..246d2b05633 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -983,6 +983,8 @@ for t in tests: plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/fs_specific -U$USERNAME%$PASSWORD', 'fs_specific') plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') + elif t == "smb2.ioctl-on-stream": + plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') elif t == "smb2.lock": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio') plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index d5ebf93bd6a..6ceaccfc7ca 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -3838,6 +3838,80 @@ static bool test_ioctl_sparse_qar_malformed(struct torture_context *torture, return true; } +bool test_ioctl_alternate_data_stream(struct torture_context *tctx) +{ + bool ret = false; + const char *fname = DNAME "\\test_stream_ioctl_dir"; + const char *sname = DNAME "\\test_stream_ioctl_dir:stream"; + NTSTATUS status; + struct smb2_create create = {}; + struct smb2_tree *tree = NULL; + struct smb2_handle h1 = {{0}}; + union smb_ioctl ioctl; + + if (!torture_smb2_connection(tctx, &tree)) { + torture_comment(tctx, "Initializing smb2 connection failed.\n"); + return false; + } + + smb2_deltree(tree, DNAME); + + status = torture_smb2_testdir(tree, DNAME, &h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir failed\n"); + + status = smb2_util_close(tree, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + create = (struct smb2_create) { + .in.desired_access = SEC_FILE_ALL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_HIDDEN, + .in.create_disposition = NTCREATEX_DISP_CREATE, + .in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + + h1 = create.out.file.handle; + status = smb2_util_close(tree, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + + create = (struct smb2_create) { + .in.desired_access = SEC_FILE_ALL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_CREATE, + .in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION, + .in.fname = sname, + }; + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h1 = create.out.file.handle; + + ZERO_STRUCT(ioctl); + ioctl.smb2.level = RAW_IOCTL_SMB2; + ioctl.smb2.in.file.handle = h1; + ioctl.smb2.in.function = FSCTL_CREATE_OR_GET_OBJECT_ID, + ioctl.smb2.in.max_output_response = 64; + ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL; + status = smb2_ioctl(tree, tctx, &ioctl.smb2); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_ioctl failed\n"); + ret = true; + +done: + + smb2_util_close(tree, h1); + smb2_deltree(tree, DNAME); + return ret; +} + /* * 2.3.57 FSCTL_SET_ZERO_DATA Request * diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index 21e03360cd9..3a519e34c95 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -182,6 +182,8 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) test_ioctl_set_sparse); torture_suite_add_simple_test(suite, "zero-data-ioctl", test_ioctl_zero_data); + torture_suite_add_simple_test(suite, "ioctl-on-stream", + test_ioctl_alternate_data_stream); torture_suite_add_suite(suite, torture_smb2_rename_init(suite)); torture_suite_add_suite(suite, torture_smb2_sharemode_init(suite)); torture_suite_add_1smb2_test(suite, "hold-oplock", test_smb2_hold_oplock); -- 2.34.1 From be80d7ad0968f4443e09a9883efabb9b2cca6505 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 10 Nov 2022 14:43:15 -0800 Subject: [PATCH 2/2] s3: smbd: Always use metadata_fsp() when processing fsctls. Currently all fsctls we implement need the base fsp, not an alternate data stream fsp. We may revisit this later if we implement fsctls that operate on an ADS. Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15236 Signed-off-by: Jeremy Allison Reviewed-by: Andrew Walker Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Mon Nov 14 18:13:31 UTC 2022 on sn-devel-184 (cherry picked from commit fa4eba131b882c3858b28f5fd9864998e19a4510) --- selftest/knownfail.d/smb2-ioctl-stream | 1 - source3/modules/vfs_default.c | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/smb2-ioctl-stream diff --git a/selftest/knownfail.d/smb2-ioctl-stream b/selftest/knownfail.d/smb2-ioctl-stream deleted file mode 100644 index 518726e8f19..00000000000 --- a/selftest/knownfail.d/smb2-ioctl-stream +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.ioctl-on-stream.ioctl-on-stream\(fileserver\) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 48ff174ebbe..91f41578f25 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1494,7 +1494,13 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, char **out_data = (char **)_out_data; NTSTATUS status; - SMB_ASSERT(!fsp_is_alternate_stream(fsp)); + /* + * Currently all fsctls operate on the base + * file if given an alternate data stream. + * Revisit this if we implement fsctls later + * that need access to the ADS handle. + */ + fsp = metadata_fsp(fsp); switch (function) { case FSCTL_SET_SPARSE: -- 2.34.1