* [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings
@ 2026-10-07 14:43 Norbert Szetei
2026-10-07 14:46 ` Norbert Szetei
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Norbert Szetei @ 2026-10-07 14:43 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig
When an exchange covers both files from offset 0 to i_disk_size,
xmi_can_exchange_reflink_flags() assumes every mapping moved and hands
the reflink flag over to the other file.
That assumption is wrong. A file can still own shared blocks past
i_disk_size, e.g. after EXCHANGE_RANGE_TO_EOF sets a smaller size
without unmapping anything. The flag is cleared anyway, and the next
write to such a block happens in place instead of CoW, corrupting the
other file that shares it.
Commit a23eca88448e ("xfs: fix exchange-range reflink flag clearing
issue with INO1_WRITTEN") fixed one instance of this. Rather than add
another special case, stop moving the flags.
xfs_exchmaps_ensure_reflink() already sets the flag on both inodes
before the exchange, so both just keep it.
Nothing sets CLEAR_INO{1,2}_REFLINK anymore. Intents logged by older
kernels may still carry them; recovery already masks them off, so they
stay in the log format mask and are ignored.
XFS_IOC_SWAPEXT has the same problem: it moves the reflink flag from
one inode to the other along with the extents. Just removing that is
not enough here: the exchange path flags both inodes up front in
xfs_exchmaps_ensure_reflink(), but swapext has no such step, so the
inode receiving the shared extents would be left without the flag.
Set it on both inodes instead, and make sure both have a CoW fork,
since an inode with the reflink flag is expected to have one.
The flag now stays until FALLOC_FL_UNSHARE_RANGE, xfs_scrub, or a
truncate to zero removes it.
Fixes: 966ceafc7a43 ("xfs: create deferred log items for file mapping exchanges")
Fixes: f0bc4d134b46 ("xfs: swap inode reflink flags when swapping inode extents")
Cc: stable@vger.kernel.org # v6.10
Assisted-by: LLM
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
v2: also handle XFS_IOC_SWAPEXT
v1: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/
Discussion: https://lore.kernel.org/linux-xfs/20261006050914.GU2705364@frogsfrogsfrogs/
fs/xfs/libxfs/xfs_exchmaps.c | 80 ++--------------------------------
fs/xfs/libxfs/xfs_log_format.h | 4 +-
fs/xfs/xfs_bmap_util.c | 15 +++----
3 files changed, 10 insertions(+), 89 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
index 6a66b6075e0a..7af8ee35c89d 100644
--- a/fs/xfs/libxfs/xfs_exchmaps.c
+++ b/fs/xfs/libxfs/xfs_exchmaps.c
@@ -49,7 +49,7 @@ struct xfs_exchmaps_adjacent {
.right2 = { .br_startblock = HOLESTARTBLOCK }, \
}
-/* Information to reset reflink flag / CoW fork state after an exchange. */
+/* Information to reset CoW fork state after an exchange. */
/*
* If the reflink flag is set on either inode, make sure it has an incore CoW
@@ -127,9 +127,7 @@ xmi_has_more_exchange_work(const struct xfs_exchmaps_intent *xmi)
static inline bool
xmi_has_postop_work(const struct xfs_exchmaps_intent *xmi)
{
- return xmi->xmi_flags & (XFS_EXCHMAPS_CLEAR_INO1_REFLINK |
- XFS_EXCHMAPS_CLEAR_INO2_REFLINK |
- __XFS_EXCHMAPS_INO2_SHORTFORM);
+ return xmi->xmi_flags & __XFS_EXCHMAPS_INO2_SHORTFORM;
}
/* Check all mappings to make sure we can actually exchange them. */
@@ -525,18 +523,6 @@ xfs_exchmaps_link_to_sf(
return error;
}
-/* Clear the reflink flag after an exchange. */
-static inline void
-xfs_exchmaps_clear_reflink(
- struct xfs_trans *tp,
- struct xfs_inode *ip)
-{
- trace_xfs_reflink_unset_inode_flag(ip);
-
- ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
- xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-}
-
/* Finish whatever work might come after an exchange operation. */
static int
xfs_exchmaps_do_postop_work(
@@ -557,16 +543,6 @@ xfs_exchmaps_do_postop_work(
return error;
}
- if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO1_REFLINK) {
- xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip1);
- xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO1_REFLINK;
- }
-
- if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO2_REFLINK) {
- xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip2);
- xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO2_REFLINK;
- }
-
return 0;
}
@@ -948,46 +924,13 @@ xfs_exchmaps_intent_destroy_cache(void)
}
/*
- * Decide if we will exchange the reflink flags between the two files after the
- * exchange. The only time we want to do this is if we're exchanging all
- * mappings under EOF and the inode reflink flags have different states.
+ * Allocate and initialize a new incore intent item from a request.
*/
-static inline bool
-xmi_can_exchange_reflink_flags(
- const struct xfs_exchmaps_req *req,
- unsigned int reflink_state)
-{
- struct xfs_mount *mp = req->ip1->i_mount;
-
- /*
- * The INO1_WRITTEN optimization can skip exchanging hole and
- * unwritten mappings, which means we cannot guarantee that all
- * shared extents actually moved to the other file. Clearing the
- * reflink flag of an inode that still holds shared extents breaks
- * the CoW write path, so refuse to exchange the flags in that case.
- */
- if (req->flags & XFS_EXCHMAPS_INO1_WRITTEN)
- return false;
-
- if (hweight32(reflink_state) != 1)
- return false;
- if (req->startoff1 != 0 || req->startoff2 != 0)
- return false;
- if (req->blockcount != XFS_B_TO_FSB(mp, req->ip1->i_disk_size))
- return false;
- if (req->blockcount != XFS_B_TO_FSB(mp, req->ip2->i_disk_size))
- return false;
- return true;
-}
-
-
-/* Allocate and initialize a new incore intent item from a request. */
struct xfs_exchmaps_intent *
xfs_exchmaps_init_intent(
const struct xfs_exchmaps_req *req)
{
struct xfs_exchmaps_intent *xmi;
- unsigned int rs = 0;
xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache,
GFP_NOFS | __GFP_NOFAIL);
@@ -1011,23 +954,6 @@ xfs_exchmaps_init_intent(
xmi->xmi_isize2 = req->ip1->i_disk_size;
}
- /* Record the state of each inode's reflink flag before the op. */
- if (xfs_is_reflink_inode(req->ip1))
- rs |= 1;
- if (xfs_is_reflink_inode(req->ip2))
- rs |= 2;
-
- /*
- * Figure out if we're clearing the reflink flags (which effectively
- * exchanges them) after the operation.
- */
- if (xmi_can_exchange_reflink_flags(req, rs)) {
- if (rs & 1)
- xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO1_REFLINK;
- if (rs & 2)
- xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO2_REFLINK;
- }
-
if (S_ISDIR(VFS_I(xmi->xmi_ip2)->i_mode) ||
S_ISLNK(VFS_I(xmi->xmi_ip2)->i_mode))
xmi->xmi_flags |= __XFS_EXCHMAPS_INO2_SHORTFORM;
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index a4e1b3eb425c..6e620380d5dc 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -927,10 +927,8 @@ struct xfs_xmi_log_format {
*/
#define XFS_EXCHMAPS_INO1_WRITTEN (1ULL << 2)
-/* Clear the reflink flag from inode1 after the operation. */
+/* No longer set, but older intents may carry them; keep them valid. */
#define XFS_EXCHMAPS_CLEAR_INO1_REFLINK (1ULL << 3)
-
-/* Clear the reflink flag from inode2 after the operation. */
#define XFS_EXCHMAPS_CLEAR_INO2_REFLINK (1ULL << 4)
#define XFS_EXCHMAPS_LOGGED_FLAGS (XFS_EXCHMAPS_ATTR_FORK | \
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 268d159339d0..865aec3ae5ab 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1544,7 +1544,6 @@ xfs_swap_extents(
struct xfs_bstat *sbp = &sxp->sx_stat;
int src_log_flags, target_log_flags;
int error = 0;
- uint64_t f;
int resblks = 0;
unsigned int flags = 0;
struct timespec64 ctime, mtime;
@@ -1700,14 +1699,12 @@ xfs_swap_extents(
if (error)
goto out_trans_cancel;
- /* Do we have to swap reflink flags? */
- if ((ip->i_diflags2 & XFS_DIFLAG2_REFLINK) ^
- (tip->i_diflags2 & XFS_DIFLAG2_REFLINK)) {
- f = ip->i_diflags2 & XFS_DIFLAG2_REFLINK;
- ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
- ip->i_diflags2 |= tip->i_diflags2 & XFS_DIFLAG2_REFLINK;
- tip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK;
- tip->i_diflags2 |= f & XFS_DIFLAG2_REFLINK;
+ /* Either inode may now own shared blocks; flag both, with CoW forks. */
+ if ((ip->i_diflags2 | tip->i_diflags2) & XFS_DIFLAG2_REFLINK) {
+ ip->i_diflags2 |= XFS_DIFLAG2_REFLINK;
+ tip->i_diflags2 |= XFS_DIFLAG2_REFLINK;
+ xfs_ifork_init_cow(ip);
+ xfs_ifork_init_cow(tip);
}
/* Swap the cow forks. */
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings 2026-10-07 14:43 [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings Norbert Szetei @ 2026-10-07 14:46 ` Norbert Szetei 2026-10-07 16:55 ` Darrick J. Wong 2026-10-07 16:54 ` Darrick J. Wong 2026-10-07 20:31 ` Dave Chinner 2 siblings, 1 reply; 5+ messages in thread From: Norbert Szetei @ 2026-10-07 14:46 UTC (permalink / raw) To: Darrick J. Wong Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig Here is a reproducer for the XFS_IOC_SWAPEXT case, since there wasn't one on the list yet, in case it's useful for fstests. $ truncate -s 512M img && mkfs.xfs -q -m rmapbt=1,reflink=1 img $ mkdir -p /tmp/m && sudo mount -o loop img /tmp/m && sudo chmod 1777 /tmp/m $ sudo xfs_io -f -c "pwrite -q -S 0x50 0 4096" /tmp/m/peer $ sudo chmod 644 /tmp/m/peer $ cd /tmp/m && ./xfs_swapext_poc On an unpatched kernel it reports peer as rewritten. With v2 applied, peer is left intact. It needs rmapbt=1. // SPDX-License-Identifier: GPL-2.0 #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> #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) typedef struct xfs_bstime { long tv_sec; __s32 tv_nsec; } xfs_bstime_t; struct xfs_bstat { __u64 bs_ino; __u16 bs_mode; __u16 bs_nlink; __u32 bs_uid; __u32 bs_gid; __u32 bs_rdev; __s32 bs_blksize; __s64 bs_size; xfs_bstime_t bs_atime; xfs_bstime_t bs_mtime; xfs_bstime_t bs_ctime; __s64 bs_blocks; __u32 bs_xflags; __s32 bs_extsize; __s32 bs_extents; __u32 bs_gen; __u16 bs_projid_lo; __u16 bs_forkoff; __u16 bs_projid_hi; __u16 bs_sick; __u16 bs_checked; unsigned char bs_pad[2]; __u32 bs_cowextsize; __u32 bs_dmevmask; __u16 bs_dmstate; __u16 bs_aextents; }; struct xfs_swapext { __s64 sx_version; #define XFS_SX_VERSION 0 __s64 sx_fdtarget; __s64 sx_fdtmp; __s64 sx_offset; __s64 sx_length; char sx_pad[16]; struct xfs_bstat sx_stat; }; #define XFS_IOC_SWAPEXT _IOWR('X', 109, struct xfs_swapext) 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(void) { struct xfs_exchange_range xr; struct file_clone_range cr; struct xfs_swapext sx; char before[65536], after[65536], *buf; struct statfs sfs; struct stat st; int d1, d2; if (statfs(".", &sfs)) return fprintf(stderr, "statfs: %m\n"), 2; blksz = sfs.f_bsize; peer = open("peer", O_RDONLY); if (peer < 0) return fprintf(stderr, "open peer: %m\n"), 2; 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; syncfs(peer); /* so FIEMAP reports peer's real block */ 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. XFS_IOC_SWAPEXT with file1 as the target. sx_offset is 0 and * sx_length equals both inodes' i_disk_size, so the "Verify all data * are being swapped" test passes. xfs_swap_extent_rmap() then bounds * its remap loop with XFS_B_TO_FSB(mp, i_size_read(VFS_I(ip))), which * is one block, so file1's two mappings above its size are left in * place -- and the reflink flags are swapped anyway, clearing file1's. */ fsync(f1); fsync(d2); if (fstat(f1, &st)) return fprintf(stderr, "fstat file1: %m\n"), 2; memset(&sx, 0, sizeof sx); sx.sx_version = XFS_SX_VERSION; sx.sx_fdtarget = f1; sx.sx_fdtmp = d2; sx.sx_offset = 0; sx.sx_length = st.st_size; sx.sx_stat.bs_mtime.tv_sec = st.st_mtim.tv_sec; sx.sx_stat.bs_mtime.tv_nsec = st.st_mtim.tv_nsec; sx.sx_stat.bs_ctime.tv_sec = st.st_ctim.tv_sec; sx.sx_stat.bs_ctime.tv_nsec = st.st_ctim.tv_nsec; if (ioctl(f1, XFS_IOC_SWAPEXT, &sx)) return fprintf(stderr, "SWAPEXT: %m\n"), 2; show("3 SWAPEXT"); /* 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; } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings 2026-10-07 14:46 ` Norbert Szetei @ 2026-10-07 16:55 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2026-10-07 16:55 UTC (permalink / raw) To: Norbert Szetei Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig On Wed, Oct 07, 2026 at 04:46:39PM +0200, Norbert Szetei wrote: > Here is a reproducer for the XFS_IOC_SWAPEXT case, since there wasn't > one on the list yet, in case it's useful for fstests. > > $ truncate -s 512M img && mkfs.xfs -q -m rmapbt=1,reflink=1 img > $ mkdir -p /tmp/m && sudo mount -o loop img /tmp/m && sudo chmod 1777 /tmp/m > $ sudo xfs_io -f -c "pwrite -q -S 0x50 0 4096" /tmp/m/peer > $ sudo chmod 644 /tmp/m/peer > $ cd /tmp/m && ./xfs_swapext_poc > > On an unpatched kernel it reports peer as rewritten. With v2 applied, > peer is left intact. It needs rmapbt=1. Thanks for the third repro, I'll kludge that into a fstests too. --D > // SPDX-License-Identifier: GPL-2.0 > #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> > > #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) > > typedef struct xfs_bstime { > long tv_sec; > __s32 tv_nsec; > } xfs_bstime_t; > > struct xfs_bstat { > __u64 bs_ino; > __u16 bs_mode; > __u16 bs_nlink; > __u32 bs_uid; > __u32 bs_gid; > __u32 bs_rdev; > __s32 bs_blksize; > __s64 bs_size; > xfs_bstime_t bs_atime; > xfs_bstime_t bs_mtime; > xfs_bstime_t bs_ctime; > __s64 bs_blocks; > __u32 bs_xflags; > __s32 bs_extsize; > __s32 bs_extents; > __u32 bs_gen; > __u16 bs_projid_lo; > __u16 bs_forkoff; > __u16 bs_projid_hi; > __u16 bs_sick; > __u16 bs_checked; > unsigned char bs_pad[2]; > __u32 bs_cowextsize; > __u32 bs_dmevmask; > __u16 bs_dmstate; > __u16 bs_aextents; > }; > > struct xfs_swapext { > __s64 sx_version; > #define XFS_SX_VERSION 0 > __s64 sx_fdtarget; > __s64 sx_fdtmp; > __s64 sx_offset; > __s64 sx_length; > char sx_pad[16]; > struct xfs_bstat sx_stat; > }; > #define XFS_IOC_SWAPEXT _IOWR('X', 109, struct xfs_swapext) > > 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(void) > { > struct xfs_exchange_range xr; > struct file_clone_range cr; > struct xfs_swapext sx; > char before[65536], after[65536], *buf; > struct statfs sfs; > struct stat st; > int d1, d2; > > if (statfs(".", &sfs)) > return fprintf(stderr, "statfs: %m\n"), 2; > blksz = sfs.f_bsize; > > peer = open("peer", O_RDONLY); > if (peer < 0) > return fprintf(stderr, "open peer: %m\n"), 2; > 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; > syncfs(peer); /* so FIEMAP reports peer's real block */ > > 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. XFS_IOC_SWAPEXT with file1 as the target. sx_offset is 0 and > * sx_length equals both inodes' i_disk_size, so the "Verify all data > * are being swapped" test passes. xfs_swap_extent_rmap() then bounds > * its remap loop with XFS_B_TO_FSB(mp, i_size_read(VFS_I(ip))), which > * is one block, so file1's two mappings above its size are left in > * place -- and the reflink flags are swapped anyway, clearing file1's. > */ > fsync(f1); > fsync(d2); > if (fstat(f1, &st)) > return fprintf(stderr, "fstat file1: %m\n"), 2; > memset(&sx, 0, sizeof sx); > sx.sx_version = XFS_SX_VERSION; > sx.sx_fdtarget = f1; > sx.sx_fdtmp = d2; > sx.sx_offset = 0; > sx.sx_length = st.st_size; > sx.sx_stat.bs_mtime.tv_sec = st.st_mtim.tv_sec; > sx.sx_stat.bs_mtime.tv_nsec = st.st_mtim.tv_nsec; > sx.sx_stat.bs_ctime.tv_sec = st.st_ctim.tv_sec; > sx.sx_stat.bs_ctime.tv_nsec = st.st_ctim.tv_nsec; > if (ioctl(f1, XFS_IOC_SWAPEXT, &sx)) > return fprintf(stderr, "SWAPEXT: %m\n"), 2; > show("3 SWAPEXT"); > > /* 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; > } > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings 2026-10-07 14:43 [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings Norbert Szetei 2026-10-07 14:46 ` Norbert Szetei @ 2026-10-07 16:54 ` Darrick J. Wong 2026-10-07 20:31 ` Dave Chinner 2 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2026-10-07 16:54 UTC (permalink / raw) To: Norbert Szetei Cc: Dave Chinner, Carlos Maiolino, linux-xfs, Christoph Hellwig On Wed, Oct 07, 2026 at 04:43:22PM +0200, Norbert Szetei wrote: > When an exchange covers both files from offset 0 to i_disk_size, > xmi_can_exchange_reflink_flags() assumes every mapping moved and hands > the reflink flag over to the other file. > > That assumption is wrong. A file can still own shared blocks past > i_disk_size, e.g. after EXCHANGE_RANGE_TO_EOF sets a smaller size > without unmapping anything. The flag is cleared anyway, and the next > write to such a block happens in place instead of CoW, corrupting the > other file that shares it. > > Commit a23eca88448e ("xfs: fix exchange-range reflink flag clearing > issue with INO1_WRITTEN") fixed one instance of this. Rather than add > another special case, stop moving the flags. > xfs_exchmaps_ensure_reflink() already sets the flag on both inodes > before the exchange, so both just keep it. > > Nothing sets CLEAR_INO{1,2}_REFLINK anymore. Intents logged by older > kernels may still carry them; recovery already masks them off, so they > stay in the log format mask and are ignored. > > XFS_IOC_SWAPEXT has the same problem: it moves the reflink flag from > one inode to the other along with the extents. Just removing that is > not enough here: the exchange path flags both inodes up front in > xfs_exchmaps_ensure_reflink(), but swapext has no such step, so the > inode receiving the shared extents would be left without the flag. > Set it on both inodes instead, and make sure both have a CoW fork, > since an inode with the reflink flag is expected to have one. > > The flag now stays until FALLOC_FL_UNSHARE_RANGE, xfs_scrub, or a > truncate to zero removes it. > > Fixes: 966ceafc7a43 ("xfs: create deferred log items for file mapping exchanges") > Fixes: f0bc4d134b46 ("xfs: swap inode reflink flags when swapping inode extents") > Cc: stable@vger.kernel.org # v6.10 > Assisted-by: LLM > Signed-off-by: Norbert Szetei <norbert@doyensec.com> Yeah, let's shut off this whole class of attacks... Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > v2: also handle XFS_IOC_SWAPEXT > v1: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/ > Discussion: https://lore.kernel.org/linux-xfs/20261006050914.GU2705364@frogsfrogsfrogs/ > > fs/xfs/libxfs/xfs_exchmaps.c | 80 ++-------------------------------- > fs/xfs/libxfs/xfs_log_format.h | 4 +- > fs/xfs/xfs_bmap_util.c | 15 +++---- > 3 files changed, 10 insertions(+), 89 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c > index 6a66b6075e0a..7af8ee35c89d 100644 > --- a/fs/xfs/libxfs/xfs_exchmaps.c > +++ b/fs/xfs/libxfs/xfs_exchmaps.c > @@ -49,7 +49,7 @@ struct xfs_exchmaps_adjacent { > .right2 = { .br_startblock = HOLESTARTBLOCK }, \ > } > > -/* Information to reset reflink flag / CoW fork state after an exchange. */ > +/* Information to reset CoW fork state after an exchange. */ > > /* > * If the reflink flag is set on either inode, make sure it has an incore CoW > @@ -127,9 +127,7 @@ xmi_has_more_exchange_work(const struct xfs_exchmaps_intent *xmi) > static inline bool > xmi_has_postop_work(const struct xfs_exchmaps_intent *xmi) > { > - return xmi->xmi_flags & (XFS_EXCHMAPS_CLEAR_INO1_REFLINK | > - XFS_EXCHMAPS_CLEAR_INO2_REFLINK | > - __XFS_EXCHMAPS_INO2_SHORTFORM); > + return xmi->xmi_flags & __XFS_EXCHMAPS_INO2_SHORTFORM; > } > > /* Check all mappings to make sure we can actually exchange them. */ > @@ -525,18 +523,6 @@ xfs_exchmaps_link_to_sf( > return error; > } > > -/* Clear the reflink flag after an exchange. */ > -static inline void > -xfs_exchmaps_clear_reflink( > - struct xfs_trans *tp, > - struct xfs_inode *ip) > -{ > - trace_xfs_reflink_unset_inode_flag(ip); > - > - ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK; > - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); > -} > - > /* Finish whatever work might come after an exchange operation. */ > static int > xfs_exchmaps_do_postop_work( > @@ -557,16 +543,6 @@ xfs_exchmaps_do_postop_work( > return error; > } > > - if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO1_REFLINK) { > - xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip1); > - xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO1_REFLINK; > - } > - > - if (xmi->xmi_flags & XFS_EXCHMAPS_CLEAR_INO2_REFLINK) { > - xfs_exchmaps_clear_reflink(tp, xmi->xmi_ip2); > - xmi->xmi_flags &= ~XFS_EXCHMAPS_CLEAR_INO2_REFLINK; > - } > - > return 0; > } > > @@ -948,46 +924,13 @@ xfs_exchmaps_intent_destroy_cache(void) > } > > /* > - * Decide if we will exchange the reflink flags between the two files after the > - * exchange. The only time we want to do this is if we're exchanging all > - * mappings under EOF and the inode reflink flags have different states. > + * Allocate and initialize a new incore intent item from a request. > */ > -static inline bool > -xmi_can_exchange_reflink_flags( > - const struct xfs_exchmaps_req *req, > - unsigned int reflink_state) > -{ > - struct xfs_mount *mp = req->ip1->i_mount; > - > - /* > - * The INO1_WRITTEN optimization can skip exchanging hole and > - * unwritten mappings, which means we cannot guarantee that all > - * shared extents actually moved to the other file. Clearing the > - * reflink flag of an inode that still holds shared extents breaks > - * the CoW write path, so refuse to exchange the flags in that case. > - */ > - if (req->flags & XFS_EXCHMAPS_INO1_WRITTEN) > - return false; > - > - if (hweight32(reflink_state) != 1) > - return false; > - if (req->startoff1 != 0 || req->startoff2 != 0) > - return false; > - if (req->blockcount != XFS_B_TO_FSB(mp, req->ip1->i_disk_size)) > - return false; > - if (req->blockcount != XFS_B_TO_FSB(mp, req->ip2->i_disk_size)) > - return false; > - return true; > -} > - > - > -/* Allocate and initialize a new incore intent item from a request. */ > struct xfs_exchmaps_intent * > xfs_exchmaps_init_intent( > const struct xfs_exchmaps_req *req) > { > struct xfs_exchmaps_intent *xmi; > - unsigned int rs = 0; > > xmi = kmem_cache_zalloc(xfs_exchmaps_intent_cache, > GFP_NOFS | __GFP_NOFAIL); > @@ -1011,23 +954,6 @@ xfs_exchmaps_init_intent( > xmi->xmi_isize2 = req->ip1->i_disk_size; > } > > - /* Record the state of each inode's reflink flag before the op. */ > - if (xfs_is_reflink_inode(req->ip1)) > - rs |= 1; > - if (xfs_is_reflink_inode(req->ip2)) > - rs |= 2; > - > - /* > - * Figure out if we're clearing the reflink flags (which effectively > - * exchanges them) after the operation. > - */ > - if (xmi_can_exchange_reflink_flags(req, rs)) { > - if (rs & 1) > - xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO1_REFLINK; > - if (rs & 2) > - xmi->xmi_flags |= XFS_EXCHMAPS_CLEAR_INO2_REFLINK; > - } > - > if (S_ISDIR(VFS_I(xmi->xmi_ip2)->i_mode) || > S_ISLNK(VFS_I(xmi->xmi_ip2)->i_mode)) > xmi->xmi_flags |= __XFS_EXCHMAPS_INO2_SHORTFORM; > diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h > index a4e1b3eb425c..6e620380d5dc 100644 > --- a/fs/xfs/libxfs/xfs_log_format.h > +++ b/fs/xfs/libxfs/xfs_log_format.h > @@ -927,10 +927,8 @@ struct xfs_xmi_log_format { > */ > #define XFS_EXCHMAPS_INO1_WRITTEN (1ULL << 2) > > -/* Clear the reflink flag from inode1 after the operation. */ > +/* No longer set, but older intents may carry them; keep them valid. */ > #define XFS_EXCHMAPS_CLEAR_INO1_REFLINK (1ULL << 3) > - > -/* Clear the reflink flag from inode2 after the operation. */ > #define XFS_EXCHMAPS_CLEAR_INO2_REFLINK (1ULL << 4) > > #define XFS_EXCHMAPS_LOGGED_FLAGS (XFS_EXCHMAPS_ATTR_FORK | \ > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index 268d159339d0..865aec3ae5ab 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1544,7 +1544,6 @@ xfs_swap_extents( > struct xfs_bstat *sbp = &sxp->sx_stat; > int src_log_flags, target_log_flags; > int error = 0; > - uint64_t f; > int resblks = 0; > unsigned int flags = 0; > struct timespec64 ctime, mtime; > @@ -1700,14 +1699,12 @@ xfs_swap_extents( > if (error) > goto out_trans_cancel; > > - /* Do we have to swap reflink flags? */ > - if ((ip->i_diflags2 & XFS_DIFLAG2_REFLINK) ^ > - (tip->i_diflags2 & XFS_DIFLAG2_REFLINK)) { > - f = ip->i_diflags2 & XFS_DIFLAG2_REFLINK; > - ip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK; > - ip->i_diflags2 |= tip->i_diflags2 & XFS_DIFLAG2_REFLINK; > - tip->i_diflags2 &= ~XFS_DIFLAG2_REFLINK; > - tip->i_diflags2 |= f & XFS_DIFLAG2_REFLINK; > + /* Either inode may now own shared blocks; flag both, with CoW forks. */ > + if ((ip->i_diflags2 | tip->i_diflags2) & XFS_DIFLAG2_REFLINK) { > + ip->i_diflags2 |= XFS_DIFLAG2_REFLINK; > + tip->i_diflags2 |= XFS_DIFLAG2_REFLINK; > + xfs_ifork_init_cow(ip); > + xfs_ifork_init_cow(tip); > } > > /* Swap the cow forks. */ > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings 2026-10-07 14:43 [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings Norbert Szetei 2026-10-07 14:46 ` Norbert Szetei 2026-10-07 16:54 ` Darrick J. Wong @ 2026-10-07 20:31 ` Dave Chinner 2 siblings, 0 replies; 5+ messages in thread From: Dave Chinner @ 2026-10-07 20:31 UTC (permalink / raw) To: Norbert Szetei Cc: Darrick J. Wong, Carlos Maiolino, linux-xfs, Christoph Hellwig On Wed, Oct 07, 2026 at 04:43:22PM +0200, Norbert Szetei wrote: > When an exchange covers both files from offset 0 to i_disk_size, > xmi_can_exchange_reflink_flags() assumes every mapping moved and hands > the reflink flag over to the other file. > > That assumption is wrong. A file can still own shared blocks past > i_disk_size, e.g. after EXCHANGE_RANGE_TO_EOF sets a smaller size > without unmapping anything. The flag is cleared anyway, and the next > write to such a block happens in place instead of CoW, corrupting the > other file that shares it. > > Commit a23eca88448e ("xfs: fix exchange-range reflink flag clearing > issue with INO1_WRITTEN") fixed one instance of this. Rather than add > another special case, stop moving the flags. > xfs_exchmaps_ensure_reflink() already sets the flag on both inodes > before the exchange, so both just keep it. > > Nothing sets CLEAR_INO{1,2}_REFLINK anymore. Intents logged by older > kernels may still carry them; recovery already masks them off, so they > stay in the log format mask and are ignored. > > XFS_IOC_SWAPEXT has the same problem: it moves the reflink flag from > one inode to the other along with the extents. Just removing that is > not enough here: the exchange path flags both inodes up front in > xfs_exchmaps_ensure_reflink(), but swapext has no such step, so the > inode receiving the shared extents would be left without the flag. > Set it on both inodes instead, and make sure both have a CoW fork, > since an inode with the reflink flag is expected to have one. > > The flag now stays until FALLOC_FL_UNSHARE_RANGE, xfs_scrub, or a > truncate to zero removes it. > > Fixes: 966ceafc7a43 ("xfs: create deferred log items for file mapping exchanges") > Fixes: f0bc4d134b46 ("xfs: swap inode reflink flags when swapping inode extents") > Cc: stable@vger.kernel.org # v6.10 > Assisted-by: LLM > Signed-off-by: Norbert Szetei <norbert@doyensec.com> > --- > v2: also handle XFS_IOC_SWAPEXT > v1: https://lore.kernel.org/linux-xfs/1E196589-DEBE-40AC-AFEA-D420DAAB067F@doyensec.com/ > Discussion: https://lore.kernel.org/linux-xfs/20261006050914.GU2705364@frogsfrogsfrogs/ Agreed, this seems like the best course of action given the recurring issues in this area. Reviewed-by: Dave Chinner <dchinner@redhat.com> -- Dave Chinner dgc@kernel.org ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-07 20:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-07 14:43 [PATCH v2] xfs: stop exchanging reflink flags when exchanging mappings Norbert Szetei 2026-10-07 14:46 ` Norbert Szetei 2026-10-07 16:55 ` Darrick J. Wong 2026-10-07 16:54 ` Darrick J. Wong 2026-10-07 20:31 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox