Wouldn't it be safer (since pcchunk->Length is a u32 to do the
following minor change to your patch
pcchunk->Length =
- cpu_to_le32(min_t(u32, len, tcon->max_bytes_chunk));
+ cpu_to_le32(min_t(u64, len, tcon->max_bytes_chunk));
Also added Cc: stable and Fixes: tags. See attached.
On Fri, May 5, 2023 at 2:48 PM Steve French <smfrench@xxxxxxxxx> wrote:
>
> Good catch
>
> On Fri, May 5, 2023, 14:31 Pawel Witek <pawel.ireneusz.witek@xxxxxxxxx> wrote:
>>
>> I've tried to copy a 5 GB file which resulted in -EINVAL (same for larger
>> files). After wiresharking the protocol I've observed that one packet
>> requests ioctl with 'Transfer Length: 0', to which the server responded
>> with an error. Some investigation showed, that this happens when
>> len=4294967296.
>>
>> On 5/5/23 18:47, Steve French wrote:
>> > since pcchunk->Length is a 32 bit field doing cpu_to_le64 seems wrong.
>> >
>> > Perhaps one option is to split this into two lines do the minimum(u64, len, tcon->max_bytes_chunk) on one line and the cpu_to_le32 of the result on the next
>> >
>> > What is "len" in the example you see failing?
>> >
>> > On Fri, May 5, 2023 at 10:15 AM Pawel Witek <pawel.ireneusz.witek@xxxxxxxxx <mailto:pawel.ireneusz.witek@xxxxxxxxx>> wrote:
>> >
>> > Change type of pcchunk->Length from u32 to u64 to match
>> > smb2_copychunk_range arguments type. Fixes the problem where performing
>> > server-side copy with CIFS_IOC_COPYCHUNK_FILE ioctl resulted in incomplete
>> > copy of large files while returning -EINVAL.
>> >
>> > Signed-off-by: Pawel Witek <pawel.ireneusz.witek@xxxxxxxxx <mailto:pawel.ireneusz.witek@xxxxxxxxx>>
>> > ---
>> > fs/cifs/smb2ops.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>> > index a81758225fcd..35c7c35882c9 100644
>> > --- a/fs/cifs/smb2ops.c
>> > +++ b/fs/cifs/smb2ops.c
>> > @@ -1682,7 +1682,7 @@ smb2_copychunk_range(const unsigned int xid,
>> > pcchunk->SourceOffset = cpu_to_le64(src_off);
>> > pcchunk->TargetOffset = cpu_to_le64(dest_off);
>> > pcchunk->Length =
>> > - cpu_to_le32(min_t(u32, len, tcon->max_bytes_chunk));
>> > + cpu_to_le64(min_t(u64, len, tcon->max_bytes_chunk));
>> >
>> > /* Request server copy to target from src identified by key */
>> > kfree(retbuf);
>> > --
>> > 2.40.1
>> >
>> >
>> >
>> >
>> > --
>> > Thanks,
>> >
>> > Steve
--
Thanks,
Steve
From 431bbb480d05cdf5d7457c2ae417af72e5cebc82 Mon Sep 17 00:00:00 2001
From: Pawel Witek <pawel.ireneusz.witek@xxxxxxxxx>
Date: Fri, 5 May 2023 17:14:59 +0200
Subject: [PATCH] cifs: fix pcchunk length type in smb2_copychunk_range
Change type of pcchunk->Length from u32 to u64 to match
smb2_copychunk_range arguments type. Fixes the problem where performing
server-side copy with CIFS_IOC_COPYCHUNK_FILE ioctl resulted in incomplete
copy of large files while returning -EINVAL.
Fixes: 9bf0c9cd4314 ("CIFS: Fix SMB2/SMB3 Copy offload support (refcopy) for large files")
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Pawel Witek <pawel.ireneusz.witek@xxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/cifs/smb2ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index a81758225fcd..a295e4c2d54e 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1682,7 +1682,7 @@ smb2_copychunk_range(const unsigned int xid,
pcchunk->SourceOffset = cpu_to_le64(src_off);
pcchunk->TargetOffset = cpu_to_le64(dest_off);
pcchunk->Length =
- cpu_to_le32(min_t(u32, len, tcon->max_bytes_chunk));
+ cpu_to_le32(min_t(u64, len, tcon->max_bytes_chunk));
/* Request server copy to target from src identified by key */
kfree(retbuf);
--
2.34.1
