| Message ID | 20230411232342.233433-3-david@fromorbit.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: a couple of syzbot fixes | expand |
On Wed, Apr 12, 2023 at 09:23:42AM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > In commit fe08cc504448 we reworked the valid superblock version > checks. If it is a V5 filesystem, it is always valid, then we > checked if the version was less than V4 (reject) and then checked > feature fields in the V4 flags to determine if it was valid. > > What we missed was that if the version is not V4 at this point, > we shoudl reject the fs. i.e. the check current treats V6+ > filesystems as if it was a v4 filesystem. Fix this. > > cc: stable@vger.kernel.org > Fixes: fe08cc504448 ("xfs: open code sb verifier feature checks") > Signed-off-by: Dave Chinner <dchinner@redhat.com> Ugh, old code... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_sb.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c > index 99cc03a298e2..ba0f17bc1dc0 100644 > --- a/fs/xfs/libxfs/xfs_sb.c > +++ b/fs/xfs/libxfs/xfs_sb.c > @@ -72,7 +72,8 @@ xfs_sb_validate_v5_features( > } > > /* > - * We support all XFS versions newer than a v4 superblock with V2 directories. > + * We current support XFS v5 formats with known features and v4 superblocks with > + * at least V2 directories. > */ > bool > xfs_sb_good_version( > @@ -86,16 +87,16 @@ xfs_sb_good_version( > if (xfs_sb_is_v5(sbp)) > return xfs_sb_validate_v5_features(sbp); > > + /* versions prior to v4 are not supported */ > + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_4) > + return false; > + > /* We must not have any unknown v4 feature bits set */ > if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) || > ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) && > (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS))) > return false; > > - /* versions prior to v4 are not supported */ > - if (XFS_SB_VERSION_NUM(sbp) < XFS_SB_VERSION_4) > - return false; > - > /* V4 filesystems need v2 directories and unwritten extents */ > if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT)) > return false; > -- > 2.39.2 >
> @@ -86,16 +87,16 @@ xfs_sb_good_version( > if (xfs_sb_is_v5(sbp)) > return xfs_sb_validate_v5_features(sbp); > > + /* versions prior to v4 are not supported */ > + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_4) > + return false; The comment is a bit confusing now. But maybe the v4 checks should move into a xfs_sb_validate_v4_features helper anyway, which would lead to a quite nice flow here: if (xfs_sb_is_v5(sbp)) return xfs_sb_validate_v5_features(sbp); if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4) return xfs_sb_validate_v4_features(sbp); return false;
On Wed, Apr 12, 2023 at 05:13:41AM -0700, Christoph Hellwig wrote: > > @@ -86,16 +87,16 @@ xfs_sb_good_version( > > if (xfs_sb_is_v5(sbp)) > > return xfs_sb_validate_v5_features(sbp); > > > > + /* versions prior to v4 are not supported */ > > + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_4) > > + return false; > > The comment is a bit confusing now. But maybe the v4 checks should > move into a xfs_sb_validate_v4_features helper anyway, which > would lead to a quite nice flow here: > > if (xfs_sb_is_v5(sbp)) > return xfs_sb_validate_v5_features(sbp); > if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4) > return xfs_sb_validate_v4_features(sbp); > return false; Sure. Care to send a patch that does that cleanup? Cheers, Dave.
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index 99cc03a298e2..ba0f17bc1dc0 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -72,7 +72,8 @@ xfs_sb_validate_v5_features( } /* - * We support all XFS versions newer than a v4 superblock with V2 directories. + * We current support XFS v5 formats with known features and v4 superblocks with + * at least V2 directories. */ bool xfs_sb_good_version( @@ -86,16 +87,16 @@ xfs_sb_good_version( if (xfs_sb_is_v5(sbp)) return xfs_sb_validate_v5_features(sbp); + /* versions prior to v4 are not supported */ + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_4) + return false; + /* We must not have any unknown v4 feature bits set */ if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) || ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) && (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS))) return false; - /* versions prior to v4 are not supported */ - if (XFS_SB_VERSION_NUM(sbp) < XFS_SB_VERSION_4) - return false; - /* V4 filesystems need v2 directories and unwritten extents */ if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT)) return false;