Re: [PATCH] xfs: fix exchange-range-to-eof file size exchange

[Dave Chinner <dgc@kernel.org>]

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