From 18fd515efba6f773a3242e78d5fa40eca4a4cfec 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 --- 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 f130d2dc3c5..cd91a7a50e6 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 182283d9c9d..67ba7b10484 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -1098,6 +1098,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 f61a2bf96f8..d7476ec6b89 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 7da737b35942bfd5c57a8e3ab42b8ff569ea2406 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 --- 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 93a56ef8019..000c23208f5 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1487,7 +1487,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