Re: [PATCH] xfs/842: verify CoW after exchangerange with FILE1_WRITTEN on shared extents
m00nb1rd lin <[email protected]> Wed, 29 Jul 2026 10:12:43 +0800
| Newsgroups | org.kernel.vger.fstests,org.kernel.vger.linux-xfs |
|---|---|
| Message-ID | <[email protected]> |
On 2026/7/28 22:59, Darrick J. Wong Wrote: > On Tue, Jul 28, 2026 at 03:16:20PM +0800, Lin Jiapeng wrote: >> When a full-file exchangerange is requested with FILE1_WRITTEN for a >> fully sparse donor file, every mapping pair is skipped, so no extents >> actually move. However, the kernel decides to swap the reflink inode >> flag based on the request geometry alone, so the target file used to >> lose its reflink flag while still owning shared extents. A subsequent >> write then took the non-reflink write path and modified the shared >> physical blocks in place, silently corrupting the other file sharing >> them: >> >> --- tests/xfs/842.out >> +++ results/xfs/842.out.bad >> @@ -1,2 +1,3 @@ >> QA output created by 842 >> +orig changed: md5 e6065c4aa2ab1603008fc18410f579d4 -> 32c5189478e6bafa1cc76423f06c88f2 (write hit shared blocks in place) >> Silence is golden >> >> This test clones a file so that both share extents, swaps the clone >> against a sparse donor with FILE1_WRITTEN, writes to the clone, and >> verifies after a mount cycle that the original file's data is intact, >> i.e. the write was redirected through CoW. >> >> The kernel fix "xfs: fix exchange-range reflink flag clearing issue >> with INO1_WRITTEN" refuses the reflink flag exchange whenever >> FILE1_WRITTEN is requested, which makes this test pass. >> >> Reported-by: Lin Jiapeng(TencentOS Red Team) <[email protected]> >> Link: https://lore.kernel.org/r/[email protected] >> Suggested-by: Christoph Hellwig <[email protected]> >> Signed-off-by: Lin Jiapeng <[email protected]> >> --- >> tests/xfs/842 | 55 +++++++++++++++++++++++++++++++++++++++++++++++ >> tests/xfs/842.out | 2 ++ >> 2 files changed, 57 insertions(+) >> create mode 100755 tests/xfs/842 >> create mode 100644 tests/xfs/842.out >> >> diff --git a/tests/xfs/842 b/tests/xfs/842 >> new file mode 100755 >> index 0000000..6d5bcab >> --- /dev/null >> +++ b/tests/xfs/842 >> @@ -0,0 +1,55 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0-or-later >> +# Copyright (c) 2026 Tencent. All Rights Reserved. >> +# >> +# FS QA Test No. 842 >> +# >> +# Make sure that a full-file exchangerange under FILE1_WRITTEN does not strip >> +# the reflink flag from a file that still owns shared extents. When the >> +# donor file is fully sparse, every mapping pair is skipped, so no extents >> +# actually move; the target file must keep its reflink flag, and a later >> +# write must go through CoW instead of modifying the shared blocks in place. >> + >> +. ./common/preamble >> +_begin_fstest auto quick fiexchange >> + >> +# Import common functions. >> +. ./common/filter >> +. ./common/reflink >> + >> +_require_xfs_io_command exchangerange >> +_require_scratch_reflink >> +_require_scratch >> + >> +_scratch_mkfs >> $seqres.full >> +_scratch_mount >> + >> +# Create the original file with a known pattern and clone it, so that both >> +# files share the same extents. >> +_pwrite_byte 0x41 0 1m $SCRATCH_MNT/orig >> $seqres.full >> +_reflink $SCRATCH_MNT/orig $SCRATCH_MNT/clone >> $seqres.full >> + >> +# Create a fully sparse donor file of the same size. >> +$XFS_IO_PROG -f -c 'truncate 1m' $SCRATCH_MNT/donor > > Hehe, this is exactly the same sequence as the testcase I wrote > yesterday afternoon, so the review is easy; > > Reviewed-by: "Darrick J. Wong" <[email protected]> > > (For extra clarity I also had done: > > # Check reflink flags > _scratch_unmount > _scratch_xfs_db -c "path /orig" -c 'print' -c "path /donor" -c 'print' | grep reflink > _scratch_mount > > for direct confirmation of the incorrect inode flag state, but the > md5sum failure suffices to prove the defect.) > > --D > >> + >> +md5_before=$(md5sum $SCRATCH_MNT/orig | awk '{print $1}') >> + >> +# Swap the clone against the sparse donor, claiming the donor is fully >> +# written (-w). Every donor mapping is a hole, so all pairs are skipped >> +# and the clone keeps its shared extents in place. >> +$XFS_IO_PROG -c "exchangerange -f -w $SCRATCH_MNT/donor" $SCRATCH_MNT/clone \ >> + >> $seqres.full >> + >> +# Overwrite part of the clone. With the reflink flag correctly retained, >> +# this must go through CoW and leave the shared blocks of orig untouched. >> +_pwrite_byte 0x42 0 64k $SCRATCH_MNT/clone >> $seqres.full >> +_scratch_cycle_mount >> + >> +md5_after=$(md5sum $SCRATCH_MNT/orig | awk '{print $1}') >> + >> +test "$md5_before" != "$md5_after" && \ >> + echo "orig changed: md5 $md5_before -> $md5_after (write hit shared blocks in place)" >> + >> +echo Silence is golden >> +status=0 >> +exit >> diff --git a/tests/xfs/842.out b/tests/xfs/842.out >> new file mode 100644 >> index 0000000..643d052 >> --- /dev/null >> +++ b/tests/xfs/842.out >> @@ -0,0 +1,2 @@ >> +QA output created by 842 >> +Silence is golden >> -- >> 2.50.1 (Apple Git-155) >> >> Good idea! I will add direct confirmation of the incorrect inode flag state in Patch v2.