Re: [PATCH v7 11/11] xfs: restart faults racing with realtime flag changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] xfs: transaction reservation mismatch in
  xfs_iomap_write_direct() on flag race
- [High] xfs: missing xfs_get_extsz_hint() recheck in
  xfs_buffered_write_iomap_begin()
--

commit b8ad8d7e86ab00a3a7fbbde54047f3e1c17b5837
Author: Chris Mason <mason@kernel.org>

xfs: restart faults racing with realtime flag changes

This commit rechecks xfs_is_zoned_inode() under the ILOCK in the write
iomap paths and returns -EAGAIN if the realtime flag changed during a
page fault.

> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index c3465204b8d52..83ff105154b55 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c

[ ... ]

> @@ -334,6 +351,11 @@ xfs_iomap_write_direct(
>  	if (error)
>  		return error;
>  
> +	if (xfs_is_zoned_inode(ip)) {
> +		error = xfs_iomap_rtflag_changed(ip);
> +		goto out_trans_cancel;
> +	}
> +

[Severity: High]
Can xfs_iomap_write_direct() still proceed with a mismatched transaction
block reservation when FS_IOC_FSSETXATTR races before
xfs_trans_alloc_inode() takes XFS_ILOCK_EXCL?

xfs_direct_write_iomap_begin() drops XFS_ILOCK before calling
xfs_iomap_write_direct(), which then reads xfs_get_extsz_hint(ip) and
XFS_IS_REALTIME_INODE(ip) unlocked to compute resaligned, dblocks and
rblocks:

fs/xfs/xfs_iomap.c:xfs_iomap_write_direct() {
	...
	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
					   xfs_get_extsz_hint(ip));
	if (unlikely(XFS_IS_REALTIME_INODE(ip))) {
		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
		rblocks = resaligned;
	} else {
		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
		rblocks = 0;
	}
	...
	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
			rblocks, force, &tp);
}

On a zoned filesystem, toggling FS_XFLAG_REALTIME on and off before
xfs_trans_alloc_inode() locks the inode leaves xfs_is_zoned_inode(ip)
false at the check above, while tp->t_blk_res only covers metadata split
blocks:

CPU0 (write fault)                CPU1 (FS_IOC_FSSETXATTR)
xfs_direct_write_iomap_begin()
  xfs_iunlock(ip, lockmode)
                                  sets XFS_DIFLAG_REALTIME
xfs_iomap_write_direct()
  XFS_IS_REALTIME_INODE(ip) == true
  dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0)
  rblocks = resaligned
                                  clears XFS_DIFLAG_REALTIME
  xfs_trans_alloc_inode()
    xfs_ilock(ip, XFS_ILOCK_EXCL)
  xfs_is_zoned_inode(ip) == false
  xfs_bmapi_write()
    xfs_bmap_btalloc()
      xfs_trans_mod_sb()
        tp->t_blk_res_used > tp->t_blk_res
          xfs_force_shutdown()    <-- forced shutdown

While FS_XFLAG_REALTIME is set at the unlocked check, passing rblocks > 0
to xfs_trans_alloc_inode() on a zoned mount also calls xfs_dec_frextents()
directly and can fail with -ENOSPC (returning SIGBUS to the fault) before
reaching the xfs_is_zoned_inode(ip) check.  The same reservation mismatch
happens in a single step if FS_IOC_FSSETXATTR changes FS_XFLAG_REALTIME on
a non-zoned realtime mount or increases ip->i_extsize in that window.

[ ... ]

> @@ -1865,6 +1896,11 @@ xfs_buffered_write_iomap_begin(
>  	if (error)
>  		return error;
>  
> +	if (xfs_is_zoned_inode(ip)) {
> +		error = xfs_iomap_rtflag_changed(ip);
> +		goto out_unlock;
> +	}
> +

[Severity: High]
Should xfs_buffered_write_iomap_begin() also recheck xfs_get_extsz_hint(ip)
after acquiring XFS_ILOCK?

xfs_buffered_write_iomap_begin() checks xfs_get_extsz_hint(ip) before
taking XFS_ILOCK because delayed allocations cannot be used with extent
size hints.  In xfs_get_extsz_hint(ip), the return value depends on
!xfs_is_always_cow_inode(ip), XFS_DIFLAG_EXTSIZE and
XFS_IS_REALTIME_INODE(ip):

CPU0 (write fault)                CPU1 (FS_IOC_FSSETXATTR)
xfs_buffered_write_iomap_begin()
  xfs_is_zoned_inode(ip) == false
  xfs_get_extsz_hint(ip) == 0
                                  sets XFS_DIFLAG_REALTIME
                                  (non-zoned RT with sb_rextsize > 1)
  xfs_ilock_for_iomap()
  xfs_is_zoned_inode(ip) == false
  xfs_bmapi_reserve_delalloc()    <-- delalloc with extsz_hint > 0

On a zoned filesystem, the same state is reached if an inode with
XFS_DIFLAG_EXTSIZE has XFS_DIFLAG_REALTIME set when xfs_get_extsz_hint(ip)
runs (which returns 0 because xfs_is_always_cow_inode(ip) is true) and
cleared before xfs_ilock_for_iomap() acquires XFS_ILOCK, or if
FS_IOC_FSSETXATTR sets XFS_DIFLAG_EXTSIZE directly.

When a 1-block fault proceeds through xfs_bmapi_reserve_delalloc() on a
realtime inode with sb_rextsize > 1, xfs_blen_to_rtbxlen(mp, 1) rounds down
to 0, creating a delalloc extent with 0 reserved realtime extents that
later trips assertions in xfs_rtallocate_align() and underflows maxlen in
xfs_rtallocate_extent_size() during writeback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110190417.32650-1-mason@kernel.org?part=11