Re: [PATCH v2] xfs: do not unshare any blocks beyond eof

["Darrick J. Wong" <djwong@kernel.org>]

[cc linux-xfs]

On Fri, Sep 27, 2024 at 02:53:44PM +0800, Julian Sun wrote:
> Attempting to unshare extents beyond EOF will trigger
> the need zeroing case, which in turn triggers a warning.
> Therefore, let's skip the unshare process if blocks are
> beyond EOF.
> 
> This patch passed the xfstests using './check -g quick', without
> causing any additional failure
> 
> Reported-and-tested-by: syzbot+296b1c84b9cbf306e5a0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=296b1c84b9cbf306e5a0
> Fixes: 32a38a499104 ("iomap: use write_begin to read pages to unshare")
> Inspired-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> ---
>  fs/xfs/xfs_iomap.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 72c981e3dc92..81a0514b8652 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -976,6 +976,7 @@ xfs_buffered_write_iomap_begin(

I'm unsure about why this correction is in
xfs_buffered_write_iomap_begin.  If extent size hints are enabled, this
function delegates to xfs_direct_write_iomap_begin, which means that
this isn't a complete fix.

Shouldn't it suffice to clamp offset/len in xfs_reflink_unshare?

--D

>  	int			error = 0;
>  	unsigned int		lockmode = XFS_ILOCK_EXCL;
>  	u64			seq;
> +	xfs_fileoff_t eof_fsb;
>  
>  	if (xfs_is_shutdown(mp))
>  		return -EIO;
> @@ -1016,6 +1017,13 @@ xfs_buffered_write_iomap_begin(
>  	if (eof)
>  		imap.br_startoff = end_fsb; /* fake hole until the end */
>  
> +	/* Don't try to unshare any blocks beyond EOF. */
> +	eof_fsb = XFS_B_TO_FSB(mp, XFS_ISIZE(ip));
> +	if (flags & IOMAP_UNSHARE && end_fsb > eof_fsb) {
> +		xfs_trim_extent(&imap, offset_fsb, eof_fsb - offset_fsb);
> +		end_fsb = eof_fsb;
> +	}
> +
>  	/* We never need to allocate blocks for zeroing or unsharing a hole. */
>  	if ((flags & (IOMAP_UNSHARE | IOMAP_ZERO)) &&
>  	    imap.br_startoff > offset_fsb) {
> @@ -1030,7 +1038,6 @@ xfs_buffered_write_iomap_begin(
>  	 */
>  	if ((flags & IOMAP_ZERO) && imap.br_startoff <= offset_fsb &&
>  	    isnullstartblock(imap.br_startblock)) {
> -		xfs_fileoff_t eof_fsb = XFS_B_TO_FSB(mp, XFS_ISIZE(ip));
>  
>  		if (offset_fsb >= eof_fsb)
>  			goto convert_delay;
> -- 
> 2.39.2
>