[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Wed Aug 11 20:03:01 UTC 2021


The branch, master has been updated
       via  c0135096807 s3: smbd: For FSCTL calls that go async, add the outstanding tevent_reqs to the aio list on the file handle.
       via  7e7ea761a37 s4: torture: Add test for smb2.ioctl.bug14769.
       via  c551d33c6bd s3: smbd: Call smbd_fsctl_torture_async_sleep() when we get FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP.
       via  0f4a8d26888 s3: smbd: Add smbd_fsctl_torture_async_sleep() server-side code.
       via  62cd95096a7 s3: libcli: Add FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP.
       via  6b6770c2ba8 s3: smbd: Split out smb2_ioctl_smbtorture() into a separate file.
      from  4354823c514 libreplace: properly execute SYS_copy_file_range check

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit c013509680742ff45b2f5965a5564015da7d466b
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 6 23:33:06 2021 -0700

    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
    RN: smbd panic on force-close share during offload write
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Wed Aug 11 20:02:57 UTC 2021 on sn-devel-184

commit 7e7ea761a37f46f758582981bc40404ffd815513
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 6 10:54:31 2021 -0700

    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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit c551d33c6bd2e74ea3a36bec5575a70d6833b98a
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Aug 5 16:07:09 2021 -0700

    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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 0f4a8d26888ec156979a00480ed9886dcac7d426
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Aug 5 16:04:38 2021 -0700

    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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 62cd95096a76d5064b105c1b4971fa3eabd5f85d
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Aug 5 11:01:44 2021 -0700

    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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 6b6770c2ba83bf25da31623443c19a8de34e5ba4
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Aug 5 13:14:16 2021 -0700

    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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 libcli/smb/smb_constants.h           |   2 +
 selftest/knownfail                   |   1 +
 source3/smbd/smb2_ioctl.c            |  83 +++----------
 source3/smbd/smb2_ioctl_private.h    |   5 +
 source3/smbd/smb2_ioctl_smbtorture.c | 230 +++++++++++++++++++++++++++++++++++
 source3/wscript_build                |   1 +
 source4/torture/smb2/ioctl.c         |  80 ++++++++++++
 7 files changed, 334 insertions(+), 68 deletions(-)
 create mode 100644 source3/smbd/smb2_ioctl_smbtorture.c


Changeset truncated at 500 lines:

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
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/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c
index d29ff5d0303..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);
@@ -381,74 +396,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..9a259fb0dbf
--- /dev/null
+++ b/source3/smbd/smb2_ioctl_smbtorture.c
@@ -0,0 +1,230 @@
+/*
+   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 <http://www.gnu.org/licenses/>.
+*/
+
+#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 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_send(
+				TALLOC_CTX *mem_ctx,
+				struct tevent_context *ev,
+				files_struct *fsp,
+				uint8_t msecs)
+{
+	struct async_sleep_state *state = NULL;
+	struct tevent_req *subreq = NULL;
+	bool ok;
+
+	subreq = tevent_req_create(mem_ctx,
+				&state,
+				struct async_sleep_state);
+	if (!subreq) {
+		return NULL;
+	}
+
+	/*
+	 * 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(subreq, NT_STATUS_NO_MEMORY);
+		return tevent_req_post(subreq, ev);
+	}
+
+	return subreq;
+}
+
+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 bool smbd_fsctl_torture_async_sleep_recv(struct tevent_req *subreq)
+{
+	tevent_req_received(subreq);
+	return true;
+}
+
+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);
+
+	smbd_fsctl_torture_async_sleep_recv(subreq);
+	TALLOC_FREE(subreq);
+
+	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);
+}
+
+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);
+
+	case FSCTL_SMBTORTURE_FSP_ASYNC_SLEEP: {
+		struct tevent_req *subreq = NULL;
+
+		/* 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);
+		}
+
+		subreq = smbd_fsctl_torture_async_sleep_send(
+						req,
+						ev,
+						state->fsp,
+						CVAL(state->in_input.data,0));
+		if (subreq == NULL) {
+			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;
+        }
+
+	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
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");
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list