From da5a0cc1cb7dc802b3133286a2a2380e54b600b6 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 (back-ported 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 a630270e5f0..9ac842b54bf 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 c9384403ba5..72fa5efee3f 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -927,6 +927,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 a4d8885fe13..2fc8201c30c 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -3839,6 +3839,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 1d9b7d5573f..f60477a6c00 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -181,6 +181,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_1smb2_test(suite, "bench-oplock", test_smb2_bench_oplock); torture_suite_add_suite(suite, torture_smb2_sharemode_init(suite)); -- 2.34.1 From 7e87012735410dea9a9a835dacb0396a92241afb 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 c6784538353..0be634ce4c2 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1392,7 +1392,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