[PATCH v2 5/6] ksmbd: implement full-file copy for AAPL ChunkCount=0 COPYCHUNK

"Gaël Blivet-Bailly" <[email protected]>
Newsgroups org.kernel.vger.linux-cifs
Message-ID <[email protected]>
From: Gael Blivet <[email protected]>

fsctl_copychunk() treats FSCTL_SRV_COPYCHUNK with ChunkCount=0 as the
standard SMB2 "query my copy limits, don't copy anything" request and
returns success without ever looking up the file handles. That's
correct for compliant SMB2 clients, but macOS Finder's Cmd+D duplicate
sends ChunkCount=0 expecting the server to copy the whole file/stream
-- so duplicated files are left at their just-created 0 bytes while
the client reports success.

Scope the full-copy fallback to AAPL-negotiated connections on a
Time Machine share (conn->is_aapl && KSMBD_SHARE_FLAG_TIME_MACHINE)
only, so standard non-Apple SMB2 clients, and AAPL-negotiated clients
on ordinary shares, keep the spec-correct query-limits behavior
unchanged.

Re-derived against the copychunk rework in
commit 8afe741dbd67 ("ksmbd: support copychunk for alternate data streams")
and commit 165e5b86fd2c ("ksmbd: handle AAPL stream copy length mismatch"):
both streams and regular files now share a single chunk_count == 0
fast path added right after src_file_size is computed, reusing the
same buffered-copy helper and vfs_copy_file_range()/COPY_FILE_SPLICE
fallback the existing per-chunk loop already uses, rather than the
separate xattr-specific get/setxattr path this used before that
rework.

ChunksWritten/ChunkBytesWritten are 0 in the response: this is a
synthesized whole-file copy, not a response to any chunk descriptor
the client actually sent (it sent none), so there's no real chunk to
report the count/size of. Only TotalBytesWritten is meaningful here.

Signed-off-by: Gael Blivet <[email protected]>
---
v1 -> v2: Re-derived against the copychunk rework that landed upstream
since v1 (commit 8afe741dbd67, commit 165e5b86fd2c), which replaced
the separate xattr-specific stream-copy path this used with a unified
chunk_count == 0 fast path shared by streams and regular files. Also
fixed ChunksWritten/ChunkBytesWritten in the response: v1 reported
ChunksWritten=1 and a nonzero ChunkBytesWritten, describing a chunk
the client never sent (it sent zero); now both are 0, matching a
value actually validated against a real client.

 fs/smb/server/smb2pdu.c | 47 +++++++++++++++++++++++---------
 fs/smb/server/vfs.c     | 59 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 13 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index ed5b132b8..612f8c8ec 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8934,23 +8934,44 @@ static int fsctl_copychunk(struct ksmbd_work *work,
 		cpu_to_le32(ksmbd_server_side_copy_max_total_size());
 
 	chunk_count = le32_to_cpu(ci_req->ChunkCount);
-	if (chunk_count == 0)
+	/*
+	 * ChunkCount=0 is the standard SMB2 "query my copy limits" request
+	 * (no data copied) -- but macOS Finder's Cmd+D duplicate sends
+	 * FSCTL_SRV_COPYCHUNK with ChunkCount=0 meaning "copy the whole
+	 * file", relying on the AAPL-negotiated server to do a full copy
+	 * instead. Keep the standard no-op behavior for everyone else.
+	 *
+	 * Gate on the TIME_MACHINE share flag, not just conn->is_aapl:
+	 * that flag alone has ambiguous provenance -- the pre-existing
+	 * narrow UniqueId=0 path can also set it on ordinary,
+	 * non-Time-Machine shares, and this series' stated design keeps
+	 * every AAPL-driven behavior opt-in per share.
+	 */
+	if (chunk_count == 0 &&
+	    !(work->conn->is_aapl &&
+	      test_share_config_flag(work->tcon->share_conf,
+				     KSMBD_SHARE_FLAG_TIME_MACHINE)))
 		goto out;
 	total_size_written = 0;
+	i = 0;
 
-	/* verify the SRV_COPYCHUNK_COPY packet */
-	if (chunk_count > ksmbd_server_side_copy_max_chunk_count() ||
-	    input_count < struct_size(ci_req, Chunks, chunk_count)) {
-		rsp->hdr.Status = STATUS_INVALID_PARAMETER;
-		return -EINVAL;
-	}
+	if (chunk_count) {
+		/* verify the SRV_COPYCHUNK_COPY packet */
+		if (chunk_count > ksmbd_server_side_copy_max_chunk_count() ||
+		    input_count < struct_size(ci_req, Chunks, chunk_count)) {
+			rsp->hdr.Status = STATUS_INVALID_PARAMETER;
+			return -EINVAL;
+		}
 
-	chunks = &ci_req->Chunks[0];
-	for (i = 0; i < chunk_count; i++) {
-		if (le32_to_cpu(chunks[i].Length) == 0 ||
-		    le32_to_cpu(chunks[i].Length) > ksmbd_server_side_copy_max_chunk_size())
-			break;
-		total_size_written += le32_to_cpu(chunks[i].Length);
+		chunks = &ci_req->Chunks[0];
+		for (i = 0; i < chunk_count; i++) {
+			if (le32_to_cpu(chunks[i].Length) == 0 ||
+			    le32_to_cpu(chunks[i].Length) > ksmbd_server_side_copy_max_chunk_size())
+				break;
+			total_size_written += le32_to_cpu(chunks[i].Length);
+		}
+	} else {
+		chunks = &ci_req->Chunks[0];
 	}
 
 	if (i < chunk_count ||
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 7f7e73915..61d02322c 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1858,6 +1858,65 @@ int ksmbd_vfs_copy_file_ranges(struct ksmbd_work *work,
 		src_file_size = i_size_read(file_inode(src_fp->filp));
 	}
 
+	/*
+	 * macOS Finder's Cmd+D duplicate sends FSCTL_SRV_COPYCHUNK with
+	 * ChunkCount=0 meaning "copy the whole file/stream", not the
+	 * standard SMB2 "query my copy limits, no data" semantics --
+	 * fsctl_copychunk() only reaches here with chunk_count == 0 for
+	 * AAPL-negotiated connections, so this doesn't affect compliant
+	 * non-Apple clients. Without this, the destination stays at its
+	 * just-created 0 bytes / empty stream: the for loop below is a
+	 * no-op when chunk_count is 0, since it never has an iteration to
+	 * treat as "copy everything".
+	 */
+	if (chunk_count == 0 && work->conn->is_aapl) {
+		loff_t off = 0;
+
+		while (off < src_file_size) {
+			size_t remaining = src_file_size - off;
+			ssize_t copied;
+
+			/* Same source/destination offset here: an in-place,
+			 * same-inode copy at matching offsets is a degenerate
+			 * no-op range, not a real overlap, but vfs_copy_file_range
+			 * still doesn't support streams -- route those (and the
+			 * same-inode case defensively) through the buffered path.
+			 */
+			if (ksmbd_stream_fd(src_fp) || ksmbd_stream_fd(dst_fp) ||
+			    file_inode(src_fp->filp) == file_inode(dst_fp->filp)) {
+				copied = ksmbd_vfs_copy_file_range_buffered(work, src_fp, dst_fp,
+									    off, off, remaining);
+			} else {
+				copied = vfs_copy_file_range(src_fp->filp, off,
+							     dst_fp->filp, off,
+							     remaining, 0);
+				if (copied == -EOPNOTSUPP || copied == -EXDEV)
+					copied = vfs_copy_file_range(src_fp->filp, off,
+								     dst_fp->filp, off,
+								     remaining,
+								     COPY_FILE_SPLICE);
+			}
+			if (copied < 0)
+				return copied;
+			if (copied == 0)
+				break;
+			off += copied;
+		}
+
+		/*
+		 * This is a synthesized whole-file copy, not a response to
+		 * any chunk descriptor the client actually sent (it sent
+		 * none -- chunk_count is 0). Report zero chunks/chunk-bytes
+		 * rather than inventing a chunk that doesn't correspond to
+		 * anything in the request; only total_size_written (bytes
+		 * actually copied) is meaningful here.
+		 */
+		*chunk_count_written = 0;
+		*chunk_size_written = 0;
+		*total_size_written = off;
+		return 0;
+	}
+
 	for (i = 0; i < chunk_count; i++) {
 		bool stream_len_mismatch = false;
 		size_t copy_len;
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.