[PATCH] copy-chunk fails if src and dst file are on different tcons
Ralph Böhme
slow at samba.org
Fri Apr 21 10:55:55 UTC 2017
Hi!
As the subject says, this fails against Samba but works against
Windows. Verified against Windows 2016.
Attached is a possible fix plus test.
Please review & push if happy. Thanks!
-slow
-------------- next part --------------
From e58ef76680d8ce49d3259e3b3d4d71270f416051 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 21 Apr 2017 11:20:36 +0200
Subject: [PATCH 1/2] s3/smbd: allow copy-chunk with src and dest file on
different tcons
We have this check in file_fsp_get()
if (smb2req->tcon->compat != fsp->conn) {
return NULL;
}
that causes copy-chunk requests to fail if source and destination file
handle are on different shares.
Windows allows such copy-chunk request, so we have to relax the above
constraint, at least for fsctl_srv_copychunk_send().
There's nothing in MS-SMB2 that mandates such a check so we might even
remove it entirely. But from a for now just allow callers to bypass the
check by adding a flag to file_fsp_get() that allows choosing behaviour.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12752
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/files.c | 11 +++++++----
source3/smbd/proto.h | 3 ++-
source3/smbd/smb2_ioctl_network_fs.c | 3 ++-
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 6d0f05b..c0e7fdf 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -623,7 +623,8 @@ files_struct *file_fsp(struct smb_request *req, uint16_t fid)
struct files_struct *file_fsp_get(struct smbd_smb2_request *smb2req,
uint64_t persistent_id,
- uint64_t volatile_id)
+ uint64_t volatile_id,
+ bool require_same_tcon)
{
struct smbXsrv_open *op;
NTSTATUS status;
@@ -648,8 +649,10 @@ struct files_struct *file_fsp_get(struct smbd_smb2_request *smb2req,
return NULL;
}
- if (smb2req->tcon->compat != fsp->conn) {
- return NULL;
+ if (require_same_tcon) {
+ if (smb2req->tcon->compat != fsp->conn) {
+ return NULL;
+ }
}
if (smb2req->session == NULL) {
@@ -684,7 +687,7 @@ struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
return smb2req->compat_chain_fsp;
}
- fsp = file_fsp_get(smb2req, persistent_id, volatile_id);
+ fsp = file_fsp_get(smb2req, persistent_id, volatile_id, true);
if (fsp == NULL) {
return NULL;
}
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 841a095..fe59825 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -397,7 +397,8 @@ void file_free(struct smb_request *req, files_struct *fsp);
files_struct *file_fsp(struct smb_request *req, uint16_t fid);
struct files_struct *file_fsp_get(struct smbd_smb2_request *smb2req,
uint64_t persistent_id,
- uint64_t volatile_id);
+ uint64_t volatile_id,
+ bool require_same_tcon);
struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
uint64_t persistent_id,
uint64_t volatile_id);
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 5b869ec..2b2cb3e 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -208,7 +208,8 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
/* persistent/volatile keys sent as the resume key */
src_persistent_h = BVAL(state->cc_copy.source_key, 0);
src_volatile_h = BVAL(state->cc_copy.source_key, 8);
- state->src_fsp = file_fsp_get(smb2req, src_persistent_h, src_volatile_h);
+ state->src_fsp = file_fsp_get(smb2req,src_persistent_h,
+ src_volatile_h, false);
if (state->src_fsp == NULL) {
DEBUG(3, ("invalid resume key in copy chunk req\n"));
state->status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
--
2.9.3
From d5907390e9431f9cbcbfe5f5a1ad4441bb410ec0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 21 Apr 2017 11:56:32 +0200
Subject: [PATCH 2/2] s4/torture: copy-chunk test with src and dest not on same
tcon
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12752
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source4/torture/smb2/ioctl.c | 97 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c
index 54a36a8..c9b41b8 100644
--- a/source4/torture/smb2/ioctl.c
+++ b/source4/torture/smb2/ioctl.c
@@ -1685,6 +1685,101 @@ static bool test_ioctl_copy_chunk_sparse_dest(struct torture_context *torture,
return true;
}
+static bool test_ioctl_copy_chunk_src_dst_not_same_tcon(
+ struct torture_context *torture,
+ struct smb2_tree *tree1)
+{
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct smb2_tree *tree2 = NULL;
+ struct smb2_handle src_h;
+ struct smb2_handle dest_h;
+ union smb_ioctl ioctl;
+ struct srv_copychunk_copy cc_copy;
+ struct srv_copychunk_rsp cc_rsp;
+ enum ndr_err_code ndr_ret;
+ struct req_resume_key_rsp res_key;
+ NTSTATUS status;
+ bool ok;
+
+ tmp_ctx = talloc_new(tree1);
+ torture_assert(torture, tmp_ctx != NULL, "talloc_new");
+
+ ok = torture_smb2_tree_connect(torture, tree1->session, tmp_ctx, &tree2);
+ torture_assert(torture, ok, "torture_smb2_tree_connect");
+
+ ok = test_setup_create_fill(torture, tree1, tmp_ctx, FNAME,
+ &src_h, 1024, SEC_RIGHTS_FILE_ALL,
+ FILE_ATTRIBUTE_NORMAL);
+ torture_assert(torture, ok, "src file create fill");
+
+ ok = test_setup_create_fill(torture, tree2, tmp_ctx, FNAME2,
+ &dest_h, 0, SEC_RIGHTS_FILE_ALL,
+ FILE_ATTRIBUTE_NORMAL);
+ torture_assert(torture, ok, "dest file create fill");
+
+ ZERO_STRUCT(ioctl);
+ ioctl.smb2.level = RAW_IOCTL_SMB2;
+ ioctl.smb2.in.file.handle = src_h;
+ ioctl.smb2.in.function = FSCTL_SRV_REQUEST_RESUME_KEY;
+ /* Allow for Key + ContextLength + Context */
+ ioctl.smb2.in.max_response_size = 32;
+ ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
+
+ status = smb2_ioctl(tree1, tmp_ctx, &ioctl.smb2);
+ torture_assert_ntstatus_ok(torture, status, "FSCTL_SRV_REQUEST_RESUME_KEY");
+
+ ndr_ret = ndr_pull_struct_blob(
+ &ioctl.smb2.out.out, tmp_ctx, &res_key,
+ (ndr_pull_flags_fn_t)ndr_pull_req_resume_key_rsp);
+ torture_assert_ndr_success(torture, ndr_ret, "ndr_pull_req_resume_key_rsp");
+
+ ZERO_STRUCT(ioctl);
+ ioctl.smb2.level = RAW_IOCTL_SMB2;
+ ioctl.smb2.in.file.handle = dest_h;
+ ioctl.smb2.in.function = FSCTL_SRV_COPYCHUNK;
+ ioctl.smb2.in.max_response_size = sizeof(struct srv_copychunk_rsp);
+ ioctl.smb2.in.flags = SMB2_IOCTL_FLAG_IS_FSCTL;
+
+ ZERO_STRUCT(cc_copy);
+ memcpy(cc_copy.source_key, res_key.resume_key,
+ ARRAY_SIZE(cc_copy.source_key));
+ cc_copy.chunk_count = 1;
+ cc_copy.chunks = talloc_zero_array(tmp_ctx, struct srv_copychunk, 1);
+ torture_assert(torture, (cc_copy.chunks != NULL), "talloc_zero_array");
+
+ /* copy all src file data (via a single chunk desc) */
+ cc_copy.chunks[0].source_off = 0;
+ cc_copy.chunks[0].target_off = 0;
+ cc_copy.chunks[0].length = 1024;
+
+ 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_struct_blob");
+
+ status = smb2_ioctl(tree2, tmp_ctx, &ioctl.smb2);
+ torture_assert_ntstatus_ok(torture, status, "FSCTL_SRV_COPYCHUNK");
+
+ 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, "ndr_pull_struct_blob");
+
+ ok = check_copy_chunk_rsp(torture, &cc_rsp,
+ 1, /* chunks written */
+ 0, /* chunk bytes unsuccessfully written */
+ 1024); /* total bytes written */
+ if (!ok) {
+ torture_fail(torture, "bad copy chunk response data");
+ }
+
+
+ smb2_util_close(tree1, src_h);
+ smb2_util_close(tree2, dest_h);
+ talloc_free(tmp_ctx);
+ return true;
+}
+
/*
* set the ioctl MaxOutputResponse size to less than
* sizeof(struct srv_copychunk_rsp)
@@ -6232,6 +6327,8 @@ struct torture_suite *torture_smb2_ioctl_init(void)
test_ioctl_copy_chunk_max_output_sz);
torture_suite_add_1smb2_test(suite, "copy_chunk_zero_length",
test_ioctl_copy_chunk_zero_length);
+ torture_suite_add_1smb2_test(suite, "copy_chunk_src_dst_not_same_tcon",
+ test_ioctl_copy_chunk_src_dst_not_same_tcon);
torture_suite_add_1smb2_test(suite, "compress_file_flag",
test_ioctl_compress_file_flag);
torture_suite_add_1smb2_test(suite, "compress_dir_inherit",
--
2.9.3
More information about the samba-technical
mailing list