Re: [PATCH v18 14/15] xfs: introduce health state for corrupted fsverity metadata

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] xfs: XFS_SICK_INO_FSVERITY is never cleared by online scrub or repair
--

commit 82a3ec6a24449be06212877f0d87b35ebb6ba704
Author: Andrey Albershteyn <aalbersh@kernel.org>

xfs: introduce health state for corrupted fsverity metadata

This commit adds the XFS_SICK_INO_FSVERITY inode health flag (and its bulkstat
counterpart XFS_BS_SICK_FSVERITY) and marks the inode sick in
xfs_fsverity_get_descriptor() when fsverity descriptor validation fails.

> diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
> index 1d45cf5789e86..932b447190da7 100644
> --- a/fs/xfs/libxfs/xfs_health.h
> +++ b/fs/xfs/libxfs/xfs_health.h
> @@ -104,6 +104,7 @@ struct xfs_rtgroup;
>  /* Don't propagate sick status to ag health summary during inactivation */
>  #define XFS_SICK_INO_FORGET	(1 << 12)
>  #define XFS_SICK_INO_DIRTREE	(1 << 13)  /* directory tree structure */
> +#define XFS_SICK_INO_FSVERITY	(1 << 14)  /* fsverity metadata */
>  
>  /* Primary evidence of health problems in a given group. */
>  #define XFS_SICK_FS_PRIMARY	(XFS_SICK_FS_COUNTERS | \
> @@ -140,7 +141,8 @@ struct xfs_rtgroup;
>  				 XFS_SICK_INO_XATTR | \
>  				 XFS_SICK_INO_SYMLINK | \
>  				 XFS_SICK_INO_PARENT | \
> -				 XFS_SICK_INO_DIRTREE)
> +				 XFS_SICK_INO_DIRTREE | \
> +				 XFS_SICK_INO_FSVERITY)
>  
>  #define XFS_SICK_INO_ZAPPED	(XFS_SICK_INO_BMBTD_ZAPPED | \
>  				 XFS_SICK_INO_BMBTA_ZAPPED | \
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> index 441251b360731..9b83a0735dc04 100644
> --- a/fs/xfs/xfs_fsverity.c
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -89,13 +89,17 @@ xfs_fsverity_get_descriptor(
>  	if (error)
>  		return error;
>  
> -	if (is_empty)
> +	if (is_empty) {
> +		xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
>  		return -ENODATA;
> +	}

[Severity: Medium]
Once xfs_fsverity_get_descriptor() sets XFS_SICK_INO_FSVERITY on an inode, how
is this sickness flag cleared when online scrub or repair runs?

When an inode has corrupted fsverity metadata, opening the file or running
online scrub/repair (XFS_SCRUB_TYPE_INODE) invokes:

xchk_prepare_iscrub() / xrep_dinode_core()
  xchk_inode_setup_verity()
    fsverity_ensure_verity_info()
      fsverity_get_descriptor()
        xfs_fsverity_get_descriptor()
          xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY)

In fs/xfs/scrub/health.c, type_to_health_flag maps XFS_SCRUB_TYPE_INODE only
to XFS_SICK_INO_CORE:

    [XFS_SCRUB_TYPE_INODE] = { XHG_INO, XFS_SICK_INO_CORE },

During online repair, xchk_inode() sees xchk_inode_verity_broken(sc->ip) and
marks XFS_SICK_INO_CORE | XFS_SICK_INO_FORGET corrupt. Then
xrep_inode_verity() repairs the inode by clearing XFS_DIFLAG2_VERITY and
S_VERITY:

fs/xfs/scrub/inode_repair.c:xrep_inode_verity() {
    if (xchk_inode_verity_broken(sc->ip)) {
        sc->ip->i_diflags2 &= ~XFS_DIFLAG2_VERITY;
        inode_set_flags(inode, 0, S_VERITY);

        xfs_trans_log_inode(sc->tp, sc->ip, XFS_ILOG_CORE);
    }
}

When xfs_scrub_metadata() re-runs xchk_inode() to verify the repair and calls
xchk_update_health(), xfs_inode_mark_healthy(sc->ip, mask) is called with
mask = XFS_SICK_INO_CORE | XFS_SICK_INO_FORGET, because neither xchk_inode()
nor xrep_inode_verity() adds XFS_SICK_INO_FSVERITY to sc->sick_mask or
sc->healthy_mask (and xfs_fsverity_delete_metadata(),
xfs_fsverity_reset_inode(), and xfs_fsverity_end_enable() do not clear it
either).

As a result, xfs_inode_mark_healthy() clears XFS_SICK_INO_CORE and
XFS_SICK_INO_FORGET while leaving XFS_SICK_INO_FSVERITY set in ip->i_sick.
Does this leave the repaired inode permanently sick in memory (preventing
xfs_reclaim_igrab() from reclaiming it and continuing to report
XFS_BS_SICK_FSVERITY in xfs_bulkstat_health()), and cause
xfs_inactive_health() to falsely mark the AG sick with XFS_SICK_AG_INODES when
the repaired inode is eventually inactivated because XFS_SICK_INO_FORGET was
cleared?

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