From 169b8ad7eb329df2c435afae2473badb84f456c1 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 6 Feb 2014 20:12:20 +0100 Subject: [PATCH 1/3] torture: add zero length FSCTL_SRV_COPYCHUNK test Windows Server 2012 returns NT_STATUS_INVALID_PARAMETER for FSCTL_SRV_COPYCHUNK requests that include a server-side copy length of zero, in line with MS-SMB2 3.3.5.15.6. We should match this behaviour, so test for it. Signed-off-by: David Disseldorp Reviewed-by: Jeremy Allison (cherry picked from commit 54d07da81e181072b530e88b42d0d0d17fe60df0) --- source4/torture/smb2/ioctl.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c index 501b233..561f45a 100644 --- a/source4/torture/smb2/ioctl.c +++ b/source4/torture/smb2/ioctl.c @@ -1543,6 +1543,68 @@ static bool test_ioctl_copy_chunk_max_output_sz(struct torture_context *torture, return true; } +static bool test_ioctl_copy_chunk_zero_length(struct torture_context *torture, + struct smb2_tree *tree) +{ + struct smb2_handle src_h; + struct smb2_handle dest_h; + NTSTATUS status; + union smb_ioctl ioctl; + union smb_fileinfo q; + TALLOC_CTX *tmp_ctx = talloc_new(tree); + struct srv_copychunk_copy cc_copy; + struct srv_copychunk_rsp cc_rsp; + enum ndr_err_code ndr_ret; + bool ok; + + ok = test_setup_copy_chunk(torture, tree, tmp_ctx, + 1, /* 1 chunk */ + &src_h, 4096, /* fill 4096 byte src file */ + SEC_RIGHTS_FILE_ALL, + &dest_h, 0, /* 0 byte dest file */ + SEC_RIGHTS_FILE_ALL, + &cc_copy, + &ioctl); + if (!ok) { + torture_fail(torture, "setup copy chunk error"); + } + + /* zero length server-side copy (via a single chunk desc) */ + cc_copy.chunks[0].source_off = 0; + cc_copy.chunks[0].target_off = 0; + cc_copy.chunks[0].length = 0; + + ndr_ret = ndr_push_struct_blob(&ioctl.smb2.in.out, tmp_ctx, + &cc_copy, + (ndr_push_flags_fn_t)ndr_push_srv_copychunk_copy); + torture_assert_ndr_success(torture, ndr_ret, + "ndr_push_srv_copychunk_copy"); + + status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2); + torture_assert_ntstatus_equal(torture, status, + NT_STATUS_INVALID_PARAMETER, + "bad zero-length chunk response"); + + ndr_ret = ndr_pull_struct_blob(&ioctl.smb2.out.out, tmp_ctx, + &cc_rsp, + (ndr_pull_flags_fn_t)ndr_pull_srv_copychunk_rsp); + torture_assert_ndr_success(torture, ndr_ret, "unmarshalling response"); + + ZERO_STRUCT(q); + q.all_info2.level = RAW_FILEINFO_SMB2_ALL_INFORMATION; + q.all_info2.in.file.handle = dest_h; + status = smb2_getinfo_file(tree, torture, &q); + torture_assert_ntstatus_ok(torture, status, "getinfo"); + + torture_assert_int_equal(torture, q.all_info2.out.size, 0, + "size after zero len clone"); + + smb2_util_close(tree, src_h); + smb2_util_close(tree, dest_h); + talloc_free(tmp_ctx); + return true; +} + /* basic testing of SMB2 ioctls */ @@ -1586,6 +1648,8 @@ struct torture_suite *torture_smb2_ioctl_init(void) test_ioctl_copy_chunk_sparse_dest); torture_suite_add_1smb2_test(suite, "copy_chunk_max_output_sz", test_ioctl_copy_chunk_max_output_sz); + torture_suite_add_1smb2_test(suite, "copy_chunk_zero_length", + test_ioctl_copy_chunk_zero_length); suite->description = talloc_strdup(suite, "SMB2-IOCTL tests"); -- 1.8.4.5 From d9faaf9541506df275d09b5604215d6bf9f65df2 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 6 Feb 2014 20:12:21 +0100 Subject: [PATCH 2/3] smbd/smb2_ioctl: fail zero length copy chunk requests As documented in MS-SMB2 3.3.5.15.6 Handling a Server-Side Data Copy Request, an invalid parameter response should be sent when: The Length value in a single chunk is greater than ServerSideCopyMaxChunkSize or *equal to zero*. We do not currently abide by the latter part of this clause. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10424 Signed-off-by: David Disseldorp Reviewed-by: Jeremy Allison (cherry picked from commit 00906f9604ad3e633e3d3cbc8d9dc4e2e305a455) --- source3/smbd/smb2_ioctl_network_fs.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c index 1e1e3e5..cf87584 100644 --- a/source3/smbd/smb2_ioctl_network_fs.c +++ b/source3/smbd/smb2_ioctl_network_fs.c @@ -46,16 +46,31 @@ static NTSTATUS copychunk_check_limits(struct srv_copychunk_copy *cc_copy) uint32_t i; uint32_t total_len = 0; + /* + * [MS-SMB2] 3.3.5.15.6 Handling a Server-Side Data Copy Request + * Send and invalid parameter response if: + * - The ChunkCount value is greater than + * ServerSideCopyMaxNumberofChunks + */ if (cc_copy->chunk_count > COPYCHUNK_MAX_CHUNKS) { return NT_STATUS_INVALID_PARAMETER; } for (i = 0; i < cc_copy->chunk_count; i++) { - if (cc_copy->chunks[i].length > COPYCHUNK_MAX_CHUNK_LEN) { + /* + * - The Length value in a single chunk is greater than + * ServerSideCopyMaxChunkSize or equal to zero. + */ + if ((cc_copy->chunks[i].length == 0) + || (cc_copy->chunks[i].length > COPYCHUNK_MAX_CHUNK_LEN)) { return NT_STATUS_INVALID_PARAMETER; } total_len += cc_copy->chunks[i].length; } + /* + * - Sum of Lengths in all chunks is greater than + * ServerSideCopyMaxDataSize + */ if (total_len > COPYCHUNK_MAX_TOTAL_LEN) { return NT_STATUS_INVALID_PARAMETER; } -- 1.8.4.5 From 847f06b129c66a856e782781a2be4396706afe4c Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 6 Feb 2014 20:12:22 +0100 Subject: [PATCH 3/3] vfs_btrfs: pass-through copy-chunk(len=0) requests Never map copy-chunk(len=0) requests to BTRFS_IOC_CLONE_RANGE ioctls. A BTRFS_IOC_CLONE_RANGE with @src_length=0 results in a clone of all data from @src_offset->EOF! BUG: https://bugzilla.samba.org/show_bug.cgi?id=10424 Signed-off-by: David Disseldorp Reviewed-by: Jeremy Allison (cherry picked from commit 3be664969d4de41ebb4778caabce8bcf5e303064) --- source3/modules/vfs_btrfs.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c index f854f2a..4ecf7ab 100644 --- a/source3/modules/vfs_btrfs.c +++ b/source3/modules/vfs_btrfs.c @@ -66,6 +66,27 @@ static struct tevent_req *btrfs_copy_chunk_send(struct vfs_handle_struct *handle } cc_state->handle = handle; + if (num == 0) { + /* + * With a @src_length of zero, BTRFS_IOC_CLONE_RANGE clones + * all data from @src_offset->EOF! This is certainly not what + * the caller expects, and not what vfs_default does. + */ + cc_state->subreq = SMB_VFS_NEXT_COPY_CHUNK_SEND(handle, + cc_state, ev, + src_fsp, + src_off, + dest_fsp, + dest_off, num); + if (tevent_req_nomem(cc_state->subreq, req)) { + return tevent_req_post(req, ev); + } + tevent_req_set_callback(cc_state->subreq, + btrfs_copy_chunk_done, + req); + return req; + } + status = vfs_stat_fsp(src_fsp); if (tevent_req_nterror(req, status)) { return tevent_req_post(req, ev); @@ -137,7 +158,6 @@ static struct tevent_req *btrfs_copy_chunk_send(struct vfs_handle_struct *handle btrfs_copy_chunk_done, req); return req; - } DEBUG(5, ("BTRFS_IOC_CLONE_RANGE returned %d\n", ret)); -- 1.8.4.5