[SCM] Samba Shared Repository - branch v4-20-test updated

Jule Anger janger at samba.org
Fri Aug 30 09:02:01 UTC 2024


The branch, v4-20-test has been updated
       via  72aa92c67d8 smb2_ioctl: fix truncated FSCTL_QUERY_ALLOCATED_RANGES responses
       via  f22c56b06ec s4:torture/smb2: test FSCTL_QUERY_ALLOCATED_RANGES truncation
      from  f7dc86c173e s3:smbd: fix NULL dereference in case of readlink failure

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-20-test


- Log -----------------------------------------------------------------
commit 72aa92c67d861a676377dd13397d797394178e90
Author: David Disseldorp <ddiss at samba.org>
Date:   Fri Aug 23 12:55:58 2024 +0000

    smb2_ioctl: fix truncated FSCTL_QUERY_ALLOCATED_RANGES responses
    
    As per MS-FSA 2.1.5.10.22 FSCTL_QUERY_ALLOCATED_RANGES, if response
    range entries exceed in_max_output, then we should respond with
    STATUS_BUFFER_OVERFLOW and a truncated output buffer.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=15699
    
    Reported-by: David Howells <dhowells at redhat.com>
    Signed-off-by: David Disseldorp <ddiss at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): David Disseldorp <ddiss at samba.org>
    Autobuild-Date(master): Wed Aug 28 08:54:11 UTC 2024 on atb-devel-224
    
    (cherry picked from commit 5e278a52646a48e3671270e5b57ec5b852f9fb4b)
    
    Autobuild-User(v4-20-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-20-test): Fri Aug 30 09:01:54 UTC 2024 on atb-devel-224

commit f22c56b06ecaabcbc8105aa186545e1e806c1899
Author: David Disseldorp <ddiss at samba.org>
Date:   Fri Aug 23 13:01:24 2024 +0000

    s4:torture/smb2: test FSCTL_QUERY_ALLOCATED_RANGES truncation
    
    FSCTL_QUERY_ALLOCATED_RANGES responses with more than one range should
    be truncated to account for a ioctl.smb2.in.max_output_response limit.
    Add a test for this.
    
    Flag the new test knownfail; fix in subsequent commit.
    
    Signed-off-by: David Disseldorp <ddiss at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    (cherry picked from commit 5cf57f1f539021f1490285516d8cfb2a2ab483e0)

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

Summary of changes:
 source3/smbd/smb2_ioctl.c         |   4 +-
 source3/smbd/smb2_ioctl_filesys.c |  54 ++++++++------
 source4/libcli/smb2/ioctl.c       |   3 +-
 source4/torture/smb2/ioctl.c      | 149 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 186 insertions(+), 24 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c
index 7d0f11df1ad..e31627126f4 100644
--- a/source3/smbd/smb2_ioctl.c
+++ b/source3/smbd/smb2_ioctl.c
@@ -268,7 +268,8 @@ static bool smbd_smb2_ioctl_is_failure(uint32_t ctl_code, NTSTATUS status,
 	if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)
 	 && ((ctl_code == FSCTL_PIPE_TRANSCEIVE)
 	  || (ctl_code == FSCTL_PIPE_PEEK)
-	  || (ctl_code == FSCTL_DFS_GET_REFERRALS))) {
+	  || (ctl_code == FSCTL_DFS_GET_REFERRALS)
+	  || (ctl_code == FSCTL_QUERY_ALLOCATED_RANGES))) {
 		return false;
 	}
 
@@ -344,6 +345,7 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq)
 		 * in:
 		 * - fsctl_dfs_get_refers()
 		 * - smbd_smb2_ioctl_pipe_read_done()
+		 * - fsctl_qar()
 		 */
 		status = NT_STATUS_BUFFER_TOO_SMALL;
 	}
diff --git a/source3/smbd/smb2_ioctl_filesys.c b/source3/smbd/smb2_ioctl_filesys.c
index 6cc53d4828e..1a8d1c2affa 100644
--- a/source3/smbd/smb2_ioctl_filesys.c
+++ b/source3/smbd/smb2_ioctl_filesys.c
@@ -3,7 +3,7 @@
    Core SMB2 server
 
    Copyright (C) Stefan Metzmacher 2009
-   Copyright (C) David Disseldorp 2013-2015
+   Copyright (C) David Disseldorp 2013-2024
 
    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
@@ -538,6 +538,7 @@ static NTSTATUS fsctl_qar_seek_fill(TALLOC_CTX *mem_ctx,
 				    struct files_struct *fsp,
 				    off_t curr_off,
 				    off_t max_off,
+				    size_t in_max_output,
 				    DATA_BLOB *qar_array_blob)
 {
 	NTSTATUS status = NT_STATUS_NOT_SUPPORTED;
@@ -578,6 +579,17 @@ static NTSTATUS fsctl_qar_seek_fill(TALLOC_CTX *mem_ctx,
 			return NT_STATUS_INTERNAL_ERROR;
 		}
 
+		if (qar_array_blob->length + sizeof(qar_buf) > in_max_output) {
+			/*
+			 * Earlier check ensures space for one range or more.
+			 * Subsequent overflow results in a truncated response.
+			 */
+			DBG_NOTICE("truncated QAR output: need > %zu, max %zu\n",
+				qar_array_blob->length + sizeof(qar_buf),
+				in_max_output);
+			return STATUS_BUFFER_OVERFLOW;
+		}
+
 		qar_buf.file_off = data_off;
 		/* + 1 to convert maximum offset to length */
 		qar_buf.len = MIN(hole_off, max_off + 1) - data_off;
@@ -652,6 +664,13 @@ static NTSTATUS fsctl_qar(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	/* must have enough space for at least one range */
+	if (in_max_output < sizeof(struct file_alloced_range_buf)) {
+		DEBUG(2, ("QAR max %lu insufficient for one range\n",
+			  (unsigned long)in_max_output));
+		return NT_STATUS_BUFFER_TOO_SMALL;
+	}
+
 	/*
 	 * Maximum offset is either the last valid offset _before_ EOF, or the
 	 * last byte offset within the requested range. -1 converts length to
@@ -687,31 +706,24 @@ static NTSTATUS fsctl_qar(TALLOC_CTX *mem_ctx,
 		status = fsctl_qar_buf_push(mem_ctx, &qar_buf, &qar_array_blob);
 	} else {
 		status = fsctl_qar_seek_fill(mem_ctx, fsp, qar_req.buf.file_off,
-					     max_off, &qar_array_blob);
-	}
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
+					     max_off, in_max_output,
+					     &qar_array_blob);
 	}
 
-	/* marshall response buffer. */
-	qar_rsp.far_buf_array = qar_array_blob;
+	if (NT_STATUS_IS_OK(status)
+	 || NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)) {
+		/* marshall response. STATUS_BUFFER_OVERFLOW=truncated */
+		qar_rsp.far_buf_array = qar_array_blob;
 
-	ndr_ret = ndr_push_struct_blob(out_output, mem_ctx, &qar_rsp,
-		(ndr_push_flags_fn_t)ndr_push_fsctl_query_alloced_ranges_rsp);
-	if (ndr_ret != NDR_ERR_SUCCESS) {
-		DEBUG(0, ("failed to marshall QAR rsp\n"));
-		return NT_STATUS_INVALID_PARAMETER;
-	}
-
-	if (out_output->length > in_max_output) {
-		DEBUG(2, ("QAR output len %lu exceeds max %lu\n",
-			  (unsigned long)out_output->length,
-			  (unsigned long)in_max_output));
-		data_blob_free(out_output);
-		return NT_STATUS_BUFFER_TOO_SMALL;
+		ndr_ret = ndr_push_struct_blob(out_output, mem_ctx, &qar_rsp,
+		 (ndr_push_flags_fn_t)ndr_push_fsctl_query_alloced_ranges_rsp);
+		if (ndr_ret != NDR_ERR_SUCCESS) {
+			DEBUG(0, ("failed to marshall QAR rsp\n"));
+			return NT_STATUS_INVALID_PARAMETER;
+		}
 	}
 
-	return NT_STATUS_OK;
+	return status;
 }
 
 static void smb2_ioctl_filesys_dup_extents_done(struct tevent_req *subreq);
diff --git a/source4/libcli/smb2/ioctl.c b/source4/libcli/smb2/ioctl.c
index fe74dfecd8e..94962691810 100644
--- a/source4/libcli/smb2/ioctl.c
+++ b/source4/libcli/smb2/ioctl.c
@@ -86,7 +86,8 @@ static bool smb2_ioctl_is_failure(uint32_t ctl_code, NTSTATUS status,
 	if (NT_STATUS_EQUAL(status, STATUS_BUFFER_OVERFLOW)
 	 && ((ctl_code == FSCTL_PIPE_TRANSCEIVE)
 	  || (ctl_code == FSCTL_PIPE_PEEK)
-	  || (ctl_code == FSCTL_DFS_GET_REFERRALS))) {
+	  || (ctl_code == FSCTL_DFS_GET_REFERRALS)
+	  || (ctl_code == FSCTL_QUERY_ALLOCATED_RANGES))) {
 		return false;
 	}
 
diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c
index beceaa5c551..7979e129ba7 100644
--- a/source4/torture/smb2/ioctl.c
+++ b/source4/torture/smb2/ioctl.c
@@ -3,7 +3,7 @@
 
    test suite for SMB2 ioctl operations
 
-   Copyright (C) David Disseldorp 2011-2016
+   Copyright (C) David Disseldorp 2011-2024
 
    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
@@ -3838,6 +3838,151 @@ static bool test_ioctl_sparse_qar_malformed(struct torture_context *torture,
 	return true;
 }
 
+static bool test_ioctl_sparse_qar_truncated(struct torture_context *torture,
+					    struct smb2_tree *tree)
+{
+	struct smb2_handle fh;
+	union smb_ioctl ioctl;
+	struct file_alloced_range_buf far_buf;
+	NTSTATUS status;
+	enum ndr_err_code ndr_ret;
+	TALLOC_CTX *tmp_ctx = talloc_new(tree);
+	bool ok;
+	struct file_alloced_range_buf far_rsp;
+
+	ok = test_setup_create_fill(torture, tree, tmp_ctx,
+				    FNAME, &fh, 0, SEC_RIGHTS_FILE_ALL,
+				    FILE_ATTRIBUTE_NORMAL);
+	torture_assert(torture, ok, "setup file");
+
+	status = test_ioctl_fs_supported(torture, tree, tmp_ctx, &fh,
+					 FILE_SUPPORTS_SPARSE_FILES, &ok);
+	torture_assert_ntstatus_ok(torture, status, "SMB2_GETINFO_FS");
+	if (!ok) {
+		smb2_util_close(tree, fh);
+		torture_skip(torture, "Sparse files not supported\n");
+	}
+
+	status = test_ioctl_sparse_req(torture, tmp_ctx, tree, fh, true);
+	torture_assert_ntstatus_ok(torture, status, "FSCTL_SET_SPARSE");
+
+	/*
+	 * Write 0 and 1M offsets as (hopefully) two separate extents.
+	 * XXX this test assumes that these ranges will be recorded as separate
+	 * FSCTL_QUERY_ALLOCATED_RANGES extents, which isn't strictly required:
+	 * the spec basically says the FS can do what it wants as long as
+	 * non-zeroed data ranges aren't reported as sparse holes.
+	 */
+	ok = write_pattern(torture, tree, tmp_ctx, fh,
+			   0,		/* off */
+			   1024,	/* len */
+			   0);		/* pattern offset */
+	torture_assert(torture, ok, "write pattern");
+	ok = write_pattern(torture, tree, tmp_ctx, fh,
+			   1024 * 1024,	/* off */
+			   1024,	/* len */
+			   0);		/* pattern offset */
+	torture_assert(torture, ok, "write pattern");
+
+	/* qar max output enough to carry one range, should be truncated */
+	ZERO_STRUCT(ioctl);
+	ioctl.smb2.level = RAW_IOCTL_SMB2;
+	ioctl.smb2.in.file.handle = fh;
+	ioctl.smb2.in.function = FSCTL_QUERY_ALLOCATED_RANGES;
+	ioctl.smb2.in.max_output_response = sizeof(struct file_alloced_range_buf);
+	ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
+
+	far_buf.file_off = 0;
+	far_buf.len = 2048 * 1024;
+	ndr_ret = ndr_push_struct_blob(&ioctl.smb2.in.out, tmp_ctx,
+				       &far_buf,
+			(ndr_push_flags_fn_t)ndr_push_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "push far ndr buf");
+
+	status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2);
+	torture_assert_ntstatus_equal(torture, status,
+				      STATUS_BUFFER_OVERFLOW, "qar truncated");
+	torture_assert_size_equal(torture,
+				  ioctl.smb2.out.out.length, sizeof(far_buf),
+				  "qar outlen");
+	ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx,
+				       &far_rsp,
+			(ndr_pull_flags_fn_t)ndr_pull_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "pull far range");
+	torture_assert_u64_equal(torture, far_rsp.file_off, 0, "far offset");
+	/* length depends on allocation behaviour of FS, so allow range */
+	torture_assert(torture, far_rsp.len >= 1024, "far len");
+
+	/* qar max output for just under 2 ranges, should be truncated */
+	ZERO_STRUCT(ioctl);
+	ioctl.smb2.level = RAW_IOCTL_SMB2;
+	ioctl.smb2.in.file.handle = fh;
+	ioctl.smb2.in.function = FSCTL_QUERY_ALLOCATED_RANGES;
+	ioctl.smb2.in.max_output_response = 31;
+	ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
+
+	far_buf.file_off = 0;
+	far_buf.len = 2048 * 1024;
+	ndr_ret = ndr_push_struct_blob(&ioctl.smb2.in.out, tmp_ctx,
+				       &far_buf,
+			(ndr_push_flags_fn_t)ndr_push_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "push far ndr buf");
+
+	status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2);
+	torture_assert_ntstatus_equal(torture, status,
+				      STATUS_BUFFER_OVERFLOW, "qar truncated");
+	torture_assert_size_equal(torture,
+				  ioctl.smb2.out.out.length, sizeof(far_buf),
+				  "qar outlen");
+	ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx,
+				       &far_rsp,
+			(ndr_pull_flags_fn_t)ndr_pull_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "pull far range");
+	torture_assert_u64_equal(torture, far_rsp.file_off, 0, "far offset");
+	torture_assert(torture, far_rsp.len >= 1024, "far len");
+
+	/* qar max output for 2 ranges, should pass */
+	ZERO_STRUCT(ioctl);
+	ioctl.smb2.level = RAW_IOCTL_SMB2;
+	ioctl.smb2.in.file.handle = fh;
+	ioctl.smb2.in.function = FSCTL_QUERY_ALLOCATED_RANGES;
+	ioctl.smb2.in.max_output_response = 32;
+	ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
+
+	far_buf.file_off = 0;
+	far_buf.len = 2048 * 1024;
+	ndr_ret = ndr_push_struct_blob(&ioctl.smb2.in.out, tmp_ctx,
+				       &far_buf,
+			(ndr_push_flags_fn_t)ndr_push_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "push far ndr buf");
+
+	status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2);
+	torture_assert_ntstatus_ok(torture, status, "qar non-truncated");
+	torture_assert_size_equal(torture,
+				  ioctl.smb2.out.out.length,
+				  2 * sizeof(far_buf), "qar outlen");
+	ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx,
+				       &far_rsp,
+			(ndr_pull_flags_fn_t)ndr_pull_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "pull far range");
+	torture_assert_u64_equal(torture, far_rsp.file_off, 0, "far offset");
+	torture_assert(torture, far_rsp.len >= 1024, "far len");
+	/* move to next buffer */
+	ioctl.smb2.out.out.data += sizeof(far_buf);
+	ioctl.smb2.out.out.length -= sizeof(far_buf);
+	ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx,
+				       &far_rsp,
+			(ndr_pull_flags_fn_t)ndr_pull_file_alloced_range_buf);
+	torture_assert_ndr_success(torture, ndr_ret, "pull far range");
+	torture_assert_u64_equal(torture, far_rsp.file_off, 1024 * 1024,
+				 "far offset");
+	torture_assert(torture, far_rsp.len >= 1024, "far len");
+
+	smb2_util_close(tree, fh);
+	talloc_free(tmp_ctx);
+	return true;
+}
+
 bool test_ioctl_alternate_data_stream(struct torture_context *tctx)
 {
 	bool ret = false;
@@ -7548,6 +7693,8 @@ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx)
 				     test_ioctl_sparse_qar);
 	torture_suite_add_1smb2_test(suite, "sparse_qar_malformed",
 				     test_ioctl_sparse_qar_malformed);
+	torture_suite_add_1smb2_test(suite, "sparse_qar_truncated",
+				     test_ioctl_sparse_qar_truncated);
 	torture_suite_add_1smb2_test(suite, "sparse_punch",
 				     test_ioctl_sparse_punch);
 	torture_suite_add_1smb2_test(suite, "sparse_hole_dealloc",


-- 
Samba Shared Repository



More information about the samba-cvs mailing list