Re: [PATCH] xfs: check v5 superblock features early

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

On Tue, Jul 28, 2026 at 10:04:35AM +0200, Christoph Hellwig wrote:
> When working on a new features that reuses the existing pad in the
> superblock, I noticed that mounting such a file system on an old kernel
> logs a rather confusing warning:
> 
>     XFS (vdc): Metadir superblock padding fields must be zero.
> 
> This is because we only validate the various feature fields in v5
> superblocks after the common superblock validation helper is called.
> 
> Fix this by calling the feature validation first.  To make this more
> obvious, rename xfs_validate_sb_read to xfs_validate_sb_features and only
> call it for v5 file systems.

Shouldn't it be called xfs_validate_v5_sb_features then?

Oh.  There's already a xfs_sb_validate_v5_features function that checks
that the v4 feature bits are set correctly for a v5 filesystem, and it
would be confusing to have both.  Could we move the code in
xfs_validate_sb_read into xfs_sb_validate_v5_features instead?

If you do that, then xfs_sb_good_version will validate the feature bit
recognition, which it doesn't do now.  I'm not sure what weird side
effects might result from that though.

> Fixes: eca383fcd63b ("xfs: refactor superblock verifiers")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Cc: <stable@vger.kernel.org> # v4.19

> ---
>  fs/xfs/libxfs/xfs_sb.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 47322adb7690..e3fc19440e0f 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -189,15 +189,12 @@ xfs_sb_version_to_features(
>  	return features;
>  }
>  
> -/* Check all the superblock fields we care about when reading one in. */
> +/* Check for compatible v5 superblock features when reading a superblock */
>  STATIC int
> -xfs_validate_sb_read(
> +xfs_validate_sb_features(
>  	struct xfs_mount	*mp,
>  	struct xfs_sb		*sbp)
>  {
> -	if (!xfs_sb_is_v5(sbp))
> -		return 0;
> -
>  	/*
>  	 * Version 5 superblock feature mask validation. Reject combinations
>  	 * the kernel cannot support up front before checking anything else.
> @@ -1118,10 +1115,12 @@ xfs_sb_read_verify(
>  	 * because _verify_common checks the on-disk values.
>  	 */
>  	__xfs_sb_from_disk(&sb, dsb, false);
> +	if (xfs_sb_is_v5(&sb)) {
> +		error = xfs_validate_sb_features(mp, &sb);

By putting xfs_validate_sb_features ahead of xfs_validate_sb_common, I
think we now validate the V5 feature bits (if the V5 versionnum is set)
before checking the superblock magic.  Right?

--D

> +		if (error)
> +			goto out_error;
> +	}
>  	error = xfs_validate_sb_common(mp, bp, &sb);
> -	if (error)
> -		goto out_error;
> -	error = xfs_validate_sb_read(mp, &sb);
>  
>  out_error:
>  	if (error == -EFSCORRUPTED || error == -EFSBADCRC)
> -- 
> 2.53.0
> 
>