On Wed, Sep 23, 2020 at 05:35:44PM -0500, Eric Sandeen wrote:
> A bug existed in the XFS reflink code between v5.1 and v5.5 in which
> the mapping for a COW IO was not trimmed to the mapping of the COW
> extent that was found. This resulted in a too-short copy, and
> corruption of other files which shared the original extent.
>
> (This happened only when extent size hints were set, which bypasses
> delalloc and led to this code path.)
>
> This was (inadvertently) fixed upstream with
>
> 36adcbace24e "xfs: fill out the srcmap in iomap_begin"
>
> and related patches which moved lots of this functionality to
> the iomap subsystem.
>
> Hence, this is a -stable only patch, targeted to fix this
> corruption vector without other major code changes.
>
> Fixes: 78f0cc9d55cb ("xfs: don't use delalloc extents for COW on files with extsize hints")
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.4.x
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
(Note: Someone please fix the typo "exent" in the subject line.)
--D
> ---
>
> I've tested this with a targeted reproducer (in next email) as well as
> with xfstests.
>
> Stable folk, not sure how to send a "stable only" patch, or if that's even
> valid. Assuming you're willing to accept it, I would still like to have
> some formal Reviewed-by's from the xfs developer community before it gets
> merged.
>
> Big thanks to Darrick & Dave for letting me whine about this bug and
> offering suggestions for testing and ultimately, a patch to test.
>
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 06b9e0aacf54..3289d0f4bb03 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1002,9 +1002,15 @@ xfs_file_iomap_begin(
> * I/O, which must be block aligned, we need to report the
> * newly allocated address. If the data fork has a hole, copy
> * the COW fork mapping to avoid allocating to the data fork.
> + *
> + * Otherwise, ensure that the imap range does not extend past
> + * the range allocated/found in cmap.
> */
> if (directio || imap.br_startblock == HOLESTARTBLOCK)
> imap = cmap;
> + else
> + xfs_trim_extent(&imap, cmap.br_startoff,
> + cmap.br_blockcount);
>
> end_fsb = imap.br_startoff + imap.br_blockcount;
> length = XFS_FSB_TO_B(mp, end_fsb) - offset;
>
