From 7ef46c1a13ae89da35a367bb5ce1ea19962444c2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 5 Aug 2021 13:14:16 -0700 Subject: [PATCH 1/6] s3: smbd: Split out smb2_ioctl_smbtorture() into a separate file. We will be adding async supporting code to this, and we don't want to clutter up smb2_ioctl.c. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- source3/smbd/smb2_ioctl.c | 68 ------------------ source3/smbd/smb2_ioctl_private.h | 5 ++ source3/smbd/smb2_ioctl_smbtorture.c | 100 +++++++++++++++++++++++++++ source3/wscript_build | 1 + 4 files changed, 106 insertions(+), 68 deletions(-) create mode 100644 source3/smbd/smb2_ioctl_smbtorture.c diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c index d29ff5d0303..8154d3d8bfa 100644 --- a/source3/smbd/smb2_ioctl.c +++ b/source3/smbd/smb2_ioctl.c @@ -381,74 +381,6 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq) } } -static struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, - struct tevent_context *ev, - struct tevent_req *req, - struct smbd_smb2_ioctl_state *state) -{ - NTSTATUS status; - bool ok; - - ok = lp_parm_bool(-1, "smbd", "FSCTL_SMBTORTURE", false); - if (!ok) { - goto not_supported; - } - - switch (ctl_code) { - case FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT: - if (state->in_input.length != 0) { - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - - state->smb2req->xconn->ack.force_unacked_timeout = true; - tevent_req_done(req); - return tevent_req_post(req, ev); - - case FSCTL_SMBTORTURE_IOCTL_RESPONSE_BODY_PADDING8: - if (state->in_input.length != 0) { - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - - if (state->in_max_output > 0) { - uint32_t size = state->in_max_output; - - state->out_output = data_blob_talloc(state, NULL, size); - if (tevent_req_nomem(state->out_output.data, req)) { - return tevent_req_post(req, ev); - } - memset(state->out_output.data, 8, size); - } - - state->body_padding = 8; - tevent_req_done(req); - return tevent_req_post(req, ev); - - case FSCTL_SMBTORTURE_GLOBAL_READ_RESPONSE_BODY_PADDING8: - if (state->in_input.length != 0) { - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - - state->smb2req->xconn->smb2.smbtorture.read_body_padding = 8; - tevent_req_done(req); - return tevent_req_post(req, ev); - default: - goto not_supported; - } - -not_supported: - if (IS_IPC(state->smbreq->conn)) { - status = NT_STATUS_FS_DRIVER_REQUIRED; - } else { - status = NT_STATUS_INVALID_DEVICE_REQUEST; - } - - tevent_req_nterror(req, status); - return tevent_req_post(req, ev); -} - static struct tevent_req *smbd_smb2_ioctl_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct smbd_smb2_request *smb2req, diff --git a/source3/smbd/smb2_ioctl_private.h b/source3/smbd/smb2_ioctl_private.h index 7a35f8f5d0b..d653c107d3d 100644 --- a/source3/smbd/smb2_ioctl_private.h +++ b/source3/smbd/smb2_ioctl_private.h @@ -52,4 +52,9 @@ struct tevent_req *smb2_ioctl_network_fs(uint32_t, struct tevent_req *, struct smbd_smb2_ioctl_state *); +struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, + struct tevent_context *ev, + struct tevent_req *req, + struct smbd_smb2_ioctl_state *state); + #endif diff --git a/source3/smbd/smb2_ioctl_smbtorture.c b/source3/smbd/smb2_ioctl_smbtorture.c new file mode 100644 index 00000000000..cc2c8ac55fe --- /dev/null +++ b/source3/smbd/smb2_ioctl_smbtorture.c @@ -0,0 +1,100 @@ +/* + Unix SMB/CIFS implementation. + Core SMB2 server + + Copyright (C) Stefan Metzmacher 2009 + Copyright (C) Jeremy Allison 2021 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "smbd/smbd.h" +#include "smbd/globals.h" +#include "../libcli/smb/smb_common.h" +#include "../lib/util/tevent_ntstatus.h" +#include "include/ntioctl.h" +#include "smb2_ioctl_private.h" +#include "librpc/gen_ndr/ioctl.h" + +#undef DBGC_CLASS +#define DBGC_CLASS DBGC_SMB2 + +struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, + struct tevent_context *ev, + struct tevent_req *req, + struct smbd_smb2_ioctl_state *state) +{ + NTSTATUS status; + bool ok; + + ok = lp_parm_bool(-1, "smbd", "FSCTL_SMBTORTURE", false); + if (!ok) { + goto not_supported; + } + + switch (ctl_code) { + case FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT: + if (state->in_input.length != 0) { + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + + state->smb2req->xconn->ack.force_unacked_timeout = true; + tevent_req_done(req); + return tevent_req_post(req, ev); + + case FSCTL_SMBTORTURE_IOCTL_RESPONSE_BODY_PADDING8: + if (state->in_input.length != 0) { + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + + if (state->in_max_output > 0) { + uint32_t size = state->in_max_output; + + state->out_output = data_blob_talloc(state, NULL, size); + if (tevent_req_nomem(state->out_output.data, req)) { + return tevent_req_post(req, ev); + } + memset(state->out_output.data, 8, size); + } + + state->body_padding = 8; + tevent_req_done(req); + return tevent_req_post(req, ev); + + case FSCTL_SMBTORTURE_GLOBAL_READ_RESPONSE_BODY_PADDING8: + if (state->in_input.length != 0) { + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + + state->smb2req->xconn->smb2.smbtorture.read_body_padding = 8; + tevent_req_done(req); + return tevent_req_post(req, ev); + default: + goto not_supported; + } + +not_supported: + if (IS_IPC(state->smbreq->conn)) { + status = NT_STATUS_FS_DRIVER_REQUIRED; + } else { + status = NT_STATUS_INVALID_DEVICE_REQUEST; + } + + tevent_req_nterror(req, status); + return tevent_req_post(req, ev); +} diff --git a/source3/wscript_build b/source3/wscript_build index 8f5ce0e99ff..71e24ff3367 100644 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -663,6 +663,7 @@ bld.SAMBA3_LIBRARY('smbd_base', smbd/smb2_ioctl_filesys.c smbd/smb2_ioctl_named_pipe.c smbd/smb2_ioctl_network_fs.c + smbd/smb2_ioctl_smbtorture.c smbd/smb2_keepalive.c smbd/smb2_query_directory.c smbd/smb2_notify.c -- 2.30.2 From c2dec1e86d37da78eda3bef934dda029f40a8bb8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 5 Aug 2021 11:01:44 -0700 Subject: [PATCH 2/6] s3: libcli: Add FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP. Prepare for async FSCTL tests on an fsp. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- libcli/smb/smb_constants.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libcli/smb/smb_constants.h b/libcli/smb/smb_constants.h index a12086e602b..a043cbc883e 100644 --- a/libcli/smb/smb_constants.h +++ b/libcli/smb/smb_constants.h @@ -599,6 +599,8 @@ enum csc_policy { (FSCTL_SMBTORTURE | FSCTL_ACCESS_WRITE | 0x0010 | FSCTL_METHOD_NEITHER) #define FSCTL_SMBTORTURE_GLOBAL_READ_RESPONSE_BODY_PADDING8 \ (FSCTL_SMBTORTURE | FSCTL_ACCESS_WRITE | 0x0020 | FSCTL_METHOD_NEITHER) +#define FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP \ + (FSCTL_SMBTORTURE | FSCTL_ACCESS_WRITE | 0x0040 | FSCTL_METHOD_NEITHER) /* * A few values from [MS-FSCC] 2.1.2.1 Reparse Tags -- 2.30.2 From 8af400604b9edca9c957a0e3c416b762c7b7a4c4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 5 Aug 2021 16:04:38 -0700 Subject: [PATCH 3/6] s3: smbd: Add smbd_fsctl_torture_async_sleep() server-side code. Commented out as not yet called. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- source3/smbd/smb2_ioctl_smbtorture.c | 98 ++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/source3/smbd/smb2_ioctl_smbtorture.c b/source3/smbd/smb2_ioctl_smbtorture.c index cc2c8ac55fe..ef49bec4c75 100644 --- a/source3/smbd/smb2_ioctl_smbtorture.c +++ b/source3/smbd/smb2_ioctl_smbtorture.c @@ -31,6 +31,104 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_SMB2 +#if 0 +struct async_sleep_state { + struct smbd_server_connection *sconn; + files_struct *fsp; +}; + +static void smbd_fsctl_torture_async_sleep_done(struct tevent_req *subreq); + +static struct tevent_req *smbd_fsctl_torture_async_sleep( + struct tevent_context *ev, + struct tevent_req *req, + files_struct *fsp, + uint8_t msecs) +{ + struct async_sleep_state *state = NULL; + struct tevent_req *subreq = NULL; + bool ok; + + subreq = tevent_req_create(req, + &state, + struct async_sleep_state); + if (!subreq) { + tevent_req_nterror(req, NT_STATUS_NO_MEMORY); + return tevent_req_post(req, ev); + } + + /* + * Store the conn separately, as the test is to + * see if fsp is still a valid pointer, so we can't + * do anything other than test it for entry in the + * open files on this server connection. + */ + state->sconn = fsp->conn->sconn; + state->fsp = fsp; + + /* + * Just wait for the specified number of micro seconds, + * to allow the client time to close fsp. + */ + ok = tevent_req_set_endtime(subreq, + ev, + timeval_current_ofs(0, msecs)); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_NO_MEMORY); + return tevent_req_post(req, ev); + } + + tevent_req_set_callback(subreq, + smbd_fsctl_torture_async_sleep_done, + req); + return req; +} + +static files_struct *find_my_fsp(struct files_struct *fsp, + void *private_data) +{ + struct files_struct *myfsp = (struct files_struct *)private_data; + + if (fsp == myfsp) { + return myfsp; + } + return NULL; +} + +static void smbd_fsctl_torture_async_sleep_done(struct tevent_req *subreq) +{ + struct files_struct *found_fsp; + struct tevent_req *req = tevent_req_callback_data( + subreq, + struct tevent_req); + struct async_sleep_state *state = tevent_req_data( + subreq, + struct async_sleep_state); + + /* Does state->fsp still exist on state->sconn ? */ + found_fsp = files_forall(state->sconn, + find_my_fsp, + state->fsp); + + if (found_fsp == NULL) { + /* + * We didn't find it - return an error to the + * smb2 ioctl request. Use NT_STATUS_FILE_CLOSED so + * the client can tell the difference between + * a bad fsp handle and + * + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 + * + * This request should block file closure until it + * has completed. + */ + tevent_req_nterror(req, NT_STATUS_FILE_CLOSED); + return; + } + tevent_req_done(req); +} +#endif + struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, struct tevent_context *ev, struct tevent_req *req, -- 2.30.2 From 5f6564f708c8226a94767fa644f84049801eb845 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 5 Aug 2021 16:07:09 -0700 Subject: [PATCH 4/6] s3: smbd: Call smbd_fsctl_torture_async_sleep() when we get FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP. Now all we need is the client-side test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- source3/smbd/smb2_ioctl_smbtorture.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/source3/smbd/smb2_ioctl_smbtorture.c b/source3/smbd/smb2_ioctl_smbtorture.c index ef49bec4c75..d74c882701d 100644 --- a/source3/smbd/smb2_ioctl_smbtorture.c +++ b/source3/smbd/smb2_ioctl_smbtorture.c @@ -31,7 +31,6 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_SMB2 -#if 0 struct async_sleep_state { struct smbd_server_connection *sconn; files_struct *fsp; @@ -127,7 +126,6 @@ static void smbd_fsctl_torture_async_sleep_done(struct tevent_req *subreq) } tevent_req_done(req); } -#endif struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, struct tevent_context *ev, @@ -182,6 +180,22 @@ struct tevent_req *smb2_ioctl_smbtorture(uint32_t ctl_code, state->smb2req->xconn->smb2.smbtorture.read_body_padding = 8; tevent_req_done(req); return tevent_req_post(req, ev); + + case FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP: + /* Data is 1 byte of CVAL stored seconds to delay for. */ + if (state->in_input.length != 1) { + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + if (state->fsp == NULL) { + tevent_req_nterror(req, NT_STATUS_INVALID_HANDLE); + return tevent_req_post(req, ev); + } + return smbd_fsctl_torture_async_sleep(ev, + req, + state->fsp, + CVAL(state->in_input.data,0)); + default: goto not_supported; } -- 2.30.2 From 3192208eb4b06d6c9cd25ed62fa7d6fe044e4467 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 6 Aug 2021 10:54:31 -0700 Subject: [PATCH 5/6] s4: torture: Add test for smb2.ioctl.bug14769. Add knownfails. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- selftest/knownfail | 1 + selftest/knownfail.d/smb2_ioctl_bug14769 | 3 + source4/torture/smb2/ioctl.c | 80 ++++++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 selftest/knownfail.d/smb2_ioctl_bug14769 diff --git a/selftest/knownfail b/selftest/knownfail index b2c09e73393..9f362c02b47 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -198,6 +198,7 @@ ^samba4.smb2.ioctl.req_two_resume_keys\(ad_dc_ntvfs\) # not supported by s4 ntvfs server ^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 ^samba3.smb2.dir.one ^samba3.smb2.dir.modify ^samba3.smb2.oplock.batch20 diff --git a/selftest/knownfail.d/smb2_ioctl_bug14769 b/selftest/knownfail.d/smb2_ioctl_bug14769 new file mode 100644 index 00000000000..783a863cc06 --- /dev/null +++ b/selftest/knownfail.d/smb2_ioctl_bug14769 @@ -0,0 +1,3 @@ +^samba3.smb2.ioctl fs_specific.bug14769\(nt4_dc\) +^samba3.smb2.ioctl.bug14769\(nt4_dc\) +^samba3.smb2.ioctl.bug14769\(ad_dc\) diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index 1de5179e336..022ea001688 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -6845,6 +6845,84 @@ static bool test_ioctl_bug14607(struct torture_context *torture, return true; } +/* + basic regression test for BUG 14769 + https://bugzilla.samba.org/show_bug.cgi?id=14769 +*/ +static bool test_ioctl_bug14769(struct torture_context *torture, + struct smb2_tree *tree) +{ + NTSTATUS status; + const char *fname = "bug14769"; + bool ret = false; + struct smb2_handle h; + struct smb2_ioctl ioctl; + struct smb2_close cl; + struct smb2_request *smb2arr[2] = { 0 }; + uint8_t tosend_msec = 200; + DATA_BLOB send_buf = { &tosend_msec, 1 }; + + /* Create a test file. */ + smb2_util_unlink(tree, fname); + status = torture_smb2_testfile(tree, fname, &h); + torture_assert_ntstatus_ok(torture, status, "create bug14769"); + + /* + * Send (not receive) the FSCTL. + * This should go async with a wait time of 200 msec. + */ + ZERO_STRUCT(ioctl); + ioctl.in.file.handle = h; + ioctl.in.function = FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP; + ioctl.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL; + ioctl.in.out = send_buf; + + smb2arr[0] = smb2_ioctl_send(tree, &ioctl); + torture_assert_goto(torture, + smb2arr[0] != NULL, + ret, + done, + "smb2_ioctl_send failed\n"); + /* Immediately send the close. */ + ZERO_STRUCT(cl); + cl.in.file.handle = h; + cl.in.flags = 0; + smb2arr[1] = smb2_close_send(tree, &cl); + torture_assert_goto(torture, + smb2arr[1] != NULL, + ret, + done, + "smb2_close_send failed\n"); + + /* Now get the FSCTL reply. */ + /* + * If we suffer from bug #14769 this will fail as + * the ioctl will return with NT_STATUS_FILE_CLOSED, + * as the close will have closed the handle without + * waiting for the ioctl to complete. The server shouldn't + * complete the close until the ioctl finishes. + */ + status = smb2_ioctl_recv(smb2arr[0], tree, &ioctl); + torture_assert_ntstatus_ok_goto(torture, + status, + ret, + done, + "smb2_ioctl_recv failed\n"); + + /* Followed by the close reply. */ + status = smb2_close_recv(smb2arr[1], &cl); + torture_assert_ntstatus_ok_goto(torture, + status, + ret, + done, + "smb2_ioctl_close failed\n"); + ret = true; + + done: + smb2_util_unlink(tree, fname); + return ret; +} + /* * testing of SMB2 ioctls */ @@ -6992,6 +7070,8 @@ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx) test_ioctl_dup_extents_dest_lck); torture_suite_add_1smb2_test(suite, "bug14607", test_ioctl_bug14607); + torture_suite_add_1smb2_test(suite, "bug14769", + test_ioctl_bug14769); suite->description = talloc_strdup(suite, "SMB2-IOCTL tests"); -- 2.30.2 From d09f5e9c42b7205aeb227b46c931387191a66cbd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 6 Aug 2021 23:33:06 -0700 Subject: [PATCH 6/6] s3: smbd: For FSCTL calls that go async, add the outstanding tevent_reqs to the aio list on the file handle. Remove knownfails. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14769 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/smb2_ioctl_bug14769 | 3 --- source3/smbd/smb2_ioctl.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/smb2_ioctl_bug14769 diff --git a/selftest/knownfail.d/smb2_ioctl_bug14769 b/selftest/knownfail.d/smb2_ioctl_bug14769 deleted file mode 100644 index 783a863cc06..00000000000 --- a/selftest/knownfail.d/smb2_ioctl_bug14769 +++ /dev/null @@ -1,3 +0,0 @@ -^samba3.smb2.ioctl fs_specific.bug14769\(nt4_dc\) -^samba3.smb2.ioctl.bug14769\(nt4_dc\) -^samba3.smb2.ioctl.bug14769\(ad_dc\) diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c index 8154d3d8bfa..136f9baeda3 100644 --- a/source3/smbd/smb2_ioctl.c +++ b/source3/smbd/smb2_ioctl.c @@ -230,6 +230,21 @@ NTSTATUS smbd_smb2_request_process_ioctl(struct smbd_smb2_request *req) if (subreq == NULL) { return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY); } + + /* + * If the FSCTL has gone async on a file handle, remember + * to add it to the list of async requests we need to wait + * for on file handle close. + */ + if (in_fsp != NULL && tevent_req_is_in_progress(subreq)) { + bool ok; + + ok = aio_add_req_to_fsp(in_fsp, subreq); + if (!ok) { + return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY); + } + } + tevent_req_set_callback(subreq, smbd_smb2_request_ioctl_done, req); return smbd_smb2_request_pending_queue(req, subreq, 1000); -- 2.30.2