Linux XFS filesystem development
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Norbert Szetei <norbert@doyensec.com>
Cc: Carlos Maiolino <cem@kernel.org>,
	linux-xfs@vger.kernel.org, Christoph Hellwig <hch@infradead.org>,
	Dave Chinner <dgc@kernel.org>
Subject: Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange
Date: Tue, 6 Oct 2026 14:47:49 -0700	[thread overview]
Message-ID: <20261006214749.GQ1615495@frogsfrogsfrogs> (raw)
In-Reply-To: <471DF337-49EB-46CE-B07F-45D09B899E8B@doyensec.com>

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:
> 

  reply	other threads:[~2026-10-06 21:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-10-06 22:43     ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261006214749.GQ1615495@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=cem@kernel.org \
    --cc=dgc@kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=norbert@doyensec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox