Re: [f2fs-dev] [PATCH v17 11/21] xfs: handle fsverity I/O in write/read path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] xfs: unconditional IOMAP_F_FSVERITY in xfs_map_blocks triggers WARN_ON_ONCE
- [Medium] xfs: IOMAP_F_SHARED state leak in xfs_zoned_buffered_write_iomap_begin
--

commit 2fc682567a88b5a6a260af60da335367a346353b
Author: Andrey Albershteyn via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>

xfs: handle fsverity I/O in write/read path

This commit adds fsverity handling to the XFS read and write paths. It
introduces a new inode flag for Merkle tree construction and passes the
IOMAP_F_FSVERITY flag to iomap so it processes fsverity metadata properly
beyond EOF.

> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index c30e688cfc9f6..19640e4e3ed12 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -172,12 +173,16 @@ xfs_map_blocks(
>  	int			retries = 0;
>  	int			error = 0;
>  	unsigned int		*seq;
> +	unsigned int		iomap_flags = 0;
>  
>  	if (xfs_is_shutdown(mp))
>  		return -EIO;
>  
>  	XFS_ERRORTAG_DELAY(mp, XFS_ERRTAG_WB_DELAY_MS);
>  
> +	if (xfs_iflags_test(ip, XFS_VERITY_CONSTRUCTION))
> +		iomap_flags |= IOMAP_F_FSVERITY;

[Severity: High]
Does this unconditionally set IOMAP_F_FSVERITY for any writeback when the
construction flag is set?

If a user mmaps a file with PROT_WRITE, calls FS_IOC_ENABLE_VERITY, and
dirties a folio below isize during Merkle tree construction, the writeback
will hit this path and apply IOMAP_F_FSVERITY. This appears to lack the
offset >= xfs_fsverity_metadata_offset(ip) restriction used elsewhere
(such as in xfs_bmapi_convert_one_delalloc()), which could trigger a
WARN_ON_ONCE(folio_pos(folio) < isize) in iomap_writepage_handle_eof()
for folios before EOF.

[ ... ]

> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 7c6238fed61e4..ccc8b8fabab3f 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
[ ... ]
> @@ -1668,7 +1674,8 @@ xfs_zoned_buffered_write_iomap_begin(
>  				smap.br_startoff + smap.br_blockcount);
>  			xfs_trim_extent(&smap, offset_fsb,
>  					end_fsb - offset_fsb);
> -			error = xfs_bmbt_to_iomap(ip, srcmap, &smap, flags, 0,
> +			error = xfs_bmbt_to_iomap(ip, srcmap, &smap, flags,
> +					iomap_flags,
>  					xfs_iomap_inode_sequence(ip, 0));
>  			if (error)
>  				goto out_unlock;
> @@ -1678,6 +1685,8 @@ xfs_zoned_buffered_write_iomap_begin(
>  	if (!ip->i_cowfp)
>  		xfs_ifork_init_cow(ip);
>  
> +	iomap_flags |= IOMAP_F_SHARED;
> +

[Severity: Medium]
Can this IOMAP_F_SHARED state leak across iterations?

By modifying the local iomap_flags variable here, it seems IOMAP_F_SHARED
is retained if the code later takes the goto restart path.

xfs_zoned_buffered_write_iomap_begin() {
restart:
    ...
    error = xfs_bmbt_to_iomap(..., iomap_flags, ...);
    ...
    iomap_flags |= IOMAP_F_SHARED;
    ...
    if ((flags & IOMAP_ZERO) && srcmap->type == IOMAP_HOLE) {
        if (filemap_range_needs_writeback(...)) {
            ...
            goto restart;
        }
    }
}

This would pollute the source map state with COW-specific flags on the
subsequent iteration.

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