On Mon, May 02, 2022 at 06:20:18PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Because stupid dumb fuzzers.
Dumb question: Should we make db_flds[] in db/sb.c (userspace) report
each individual feature flag as a field_t? I've been wondering why none
of my fuzz tests ever found these problems, and it's probably because
it never hit the magic bits that $scriptkiddie happened to hit.
Modulo hch's comments,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_sb.c | 67 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 57 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index ec6eec5c0e02..d1afe0d43d7f 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -30,6 +30,46 @@
> * Physical superblock buffer manipulations. Shared with libxfs in userspace.
> */
>
> +/*
> + * Validate all the compulsory V4 feature bits are set on a V5 filesystem.
> + */
> +bool
> +xfs_sb_validate_v5_features(
> + struct xfs_sb *sbp)
> +{
> + /* We must not have any unknown V4 feature bits set */
> + if (sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS)
> + return false;
> +
> + /*
> + * The CRC bit is considered an invalid V4 flag, so we have to add it
> + * manually to the OKBITS mask.
> + */
> + if (sbp->sb_features2 & ~(XFS_SB_VERSION2_OKBITS |
> + XFS_SB_VERSION2_CRCBIT))
> + return false;
> +
> + /* Now check all the required V4 feature flags are set. */
> +
> +#define V5_VERS_FLAGS (XFS_SB_VERSION_NLINKBIT | \
> + XFS_SB_VERSION_ALIGNBIT | \
> + XFS_SB_VERSION_LOGV2BIT | \
> + XFS_SB_VERSION_EXTFLGBIT | \
> + XFS_SB_VERSION_DIRV2BIT | \
> + XFS_SB_VERSION_MOREBITSBIT)
> +
> +#define V5_FEAT_FLAGS (XFS_SB_VERSION2_LAZYSBCOUNTBIT | \
> + XFS_SB_VERSION2_ATTR2BIT | \
> + XFS_SB_VERSION2_PROJID32BIT | \
> + XFS_SB_VERSION2_CRCBIT)
> +
> + if ((sbp->sb_versionnum & V5_VERS_FLAGS) != V5_VERS_FLAGS)
> + return false;
> + if ((sbp->sb_features2 & V5_FEAT_FLAGS) != V5_FEAT_FLAGS)
> + return false;
> + return true;
> +}
> +
> /*
> * We support all XFS versions newer than a v4 superblock with V2 directories.
> */
> @@ -37,9 +77,19 @@ bool
> xfs_sb_good_version(
> struct xfs_sb *sbp)
> {
> - /* all v5 filesystems are supported */
> + /*
> + * All v5 filesystems are supported, but we must check that all the
> + * required v4 feature flags are enabled correctly as the code checks
> + * those flags and not for v5 support.
> + */
> if (xfs_sb_is_v5(sbp))
> - return true;
> + return xfs_sb_validate_v5_features(sbp);
> +
> + /* 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)
> @@ -51,12 +101,6 @@ xfs_sb_good_version(
> if (!(sbp->sb_versionnum & XFS_SB_VERSION_EXTFLGBIT))
> return false;
>
> - /* And 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;
> -
> /* It's a supported v4 filesystem */
> return true;
> }
> @@ -267,12 +311,15 @@ xfs_validate_sb_common(
> bool has_dalign;
>
> if (!xfs_verify_magic(bp, dsb->sb_magicnum)) {
> - xfs_warn(mp, "bad magic number");
> + xfs_warn(mp,
> +"Superblock has bad magic number 0x%x. Not an XFS filesystem?",
> + be32_to_cpu(dsb->sb_magicnum));
> return -EWRONGFS;
> }
>
> if (!xfs_sb_good_version(sbp)) {
> - xfs_warn(mp, "bad version");
> + xfs_warn(mp,
> +"Superblock has unknown features enabled or corrupted feature masks.");
> return -EWRONGFS;
> }
>
> --
> 2.35.1
>
