You're right, that does look better and safer :) On 5/6/23 06:51, Steve French wrote: > I was also able to reproduce it by using xfs_io to execute copy_range > on a file of exactly 4GB and verified that the patches (both versions) > fix it. > > On Fri, May 5, 2023 at 11:49 PM Steve French <smfrench@xxxxxxxxx> wrote: >> >> 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 > > >
