Re: [PATCH v3 4/7] smb/client: fix data corruption in emulated insert range

[Paulo Alcantara <pc@manguebit.org>]

Hi Huiwen,

Huiwen He <huiwen.he@linux.dev> writes:

> From: Huiwen He <hehuiwen@kylinos.cn>
>
> smb3_insert_range() shifts [off, EOF) right with COPYCHUNK, copying from
> low to high offsets. When the ranges overlap, the copy can overwrite
> source data that has not yet been copied. For a 1 MiB insert at offset 0:
>
>   offset:    0       1M      2M      3M      4M      5M
>   before:   |   A   |   B   |   C   |   D   |
>   expected: | hole  |   A   |   B   |   C   |   D   |
>   current:  | hole  |   A   |   A   |   A   |   A   | (corrupted)
>
> Let x be the insertion offset, L the total length to move, delta the
> insert length, and C the normal chunk size allowed by the server.
> Insert range maps
>
>   [x, x + L) -> [x + delta, x + delta + L).
>
> When delta >= L, the complete source and target ranges are disjoint, so
> the normal copy order and chunk size are safe:
>
>   offset: 0       4       8      12      16      20      24      28      32
>   source: [--S0--][--S1--][--S2--][--S3--]
>   target:                                 [--T0--][--T1--][--T2--][--T3--]
>
> When delta < L, the complete source and target ranges overlap, so the
> copy must proceed from EOF backwards. There are two subcases.
>
> If C <= delta, each corresponding source and target chunk is disjoint.
> The 1 MiB example has L = 4 MiB and delta = C = 1 MiB:
>
>   offset: 0       1M      2M      3M      4M      5M
>   source: [--S0--][--S1--][--S2--][--S3--]
>   target:         [--T0--][--T1--][--T2--][--T3--]
>
> Copying S0 from [0, 1M) to [1M, 2M) overwrites S1 before it is copied.
> Processing chunks from EOF backwards prevents this inter-chunk
> overwrite.
>
> If delta < C, the source and target ranges of a normal chunk also
> overlap. For example, with L = 16, delta = 2 and C = 4:
>
>   offset: 0   2   4   6   8  10  12  14  16  18
>   source: [--S0--][--S1--][--S2--][--S3--]
>   target:     [--T0--][--T1--][--T2--][--T3--]
>
> Here S0 and T0 overlap over [2,4), S1 and T1 over [6,8), and so on.
> Backward ordering cannot control how the server copies bytes inside one
> descriptor, so the chunk size must be limited to delta.
>
> Fix this by copying overlapping right shifts from EOF backwards. Limit
> the chunk size to delta when delta < C so that each chunk's source and
> target ranges do not overlap. Reject insert lengths below 4 KiB when
> this limit is needed to avoid excessive COPYCHUNK requests.
>
> Therefore:
>
>   delta >= L:
>     keep the normal copy order and chunk size
>
>   delta < L:
>     delta >=C: copy backwards and keep the normal chunk size
>     delta < C: copy backwards and limit the chunk size to delta
>
> Only the delta < C subcase requires reducing the chunk size for data
> integrity.
>
> Reproducer:
>
>   bash -c '
>           MNT=/mnt/scratch
>
>           # Generate four 1 MiB random blocks: [A][B][C][D].
>           dd if=/dev/urandom of=/tmp/src bs=1M count=4 status=none
>
>           # With C = 1 MiB, test delta = C and delta < C.
>           for delta in 1M 4K; do
>                   truncate -s 0 /tmp/expected
>                   truncate -s "$delta" /tmp/expected
>                   cat /tmp/src >> /tmp/expected
>
>                   cp /tmp/src "$MNT/file"
>                   fallocate --insert-range -o 0 -l "$delta" "$MNT/file"
>
>                   if cmp -s /tmp/expected "$MNT/file"; then
>                           echo "delta=$delta: OK"
>                   else
>                           echo "delta=$delta: CORRUPTED"
>                   fi
>           done
>   '

Without this patch and running above reproducer against Windows Server
2022, I get no data corruption.  Samba requires this patch to make it
work, though.

Besides, after applying this patch, generic/064 fails differently when
running against samba-4.24.5-1.fc44.x86_64

SECTION       -- smb3
FSTYP         -- cifs
PLATFORM      -- Linux/x86_64 fed 7.2.0 #4 SMP PREEMPT_DYNAMIC Wed Aug 26 21:42:48 -03 2026
MKFS_OPTIONS  -- //192.168.124.43/scratch
MOUNT_OPTIONS -- -ousername=testuser,password=foo-321,vers=3.1.1,mfsymlinks,noperm //192.168.124.43/scratch /mnt/scratch

generic/064        [not run] xfs_io finsert  failed (old kernel/wrong fs/bad args?)
Ran: generic/064
Not run: generic/064
Passed all 1 tests

Could you please verify?

Thanks.