[PATCH] xfs: check v5 superblock features early

[PATCH] xfs: check v5 superblock features early

[Christoph Hellwig]

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.

Fixes: eca383fcd63b ("xfs: refactor superblock verifiers")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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);
+		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

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

[Darrick J. Wong]

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
> 
> 

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

[Christoph Hellwig]

On Tue, Jul 28, 2026 at 08:26:28AM -0700, Darrick J. Wong wrote:
> 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.

Or just stick to the old name :)

> Could we move the code in
> xfs_validate_sb_read into xfs_sb_validate_v5_features instead?

But yes, that does look sensible as well.

> 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.

None in the kernel, but userspace has a lot more callers that need a
careful look.

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

[Darrick J. Wong]

On Tue, Jul 28, 2026 at 05:42:07PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 28, 2026 at 08:26:28AM -0700, Darrick J. Wong wrote:
> > 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.
> 
> Or just stick to the old name :)
> 
> > Could we move the code in
> > xfs_validate_sb_read into xfs_sb_validate_v5_features instead?
> 
> But yes, that does look sensible as well.
> 
> > 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.
> 
> None in the kernel, but userspace has a lot more callers that need a
> careful look.

I /think/ the only visible change to userspace is that the code in
repair that checks v5 feature flag recognition now becomes defunct
because libxfs_sb_good_version will catch it first.  But the
xfs_validate_sb_read code will already log (more or less) the same
complaint so I don't think it will have any real effect.

--D