Re: [PATCH 4/6] xfs: fix zero byte checking in the superblock scrubber

Christoph Hellwig
2024-12-04T08:27:39Z

On Tue, Dec 03, 2024 at 07:03:16PM -0800, Darrick J. Wong wrote:
> +/* Calculate the ondisk superblock size in bytes */
> +STATIC size_t
> +xchk_superblock_ondisk_size(
> + struct xfs_mount *mp)
> +{
> + if (xfs_has_metadir(mp))
> + return offsetofend(struct xfs_dsb, sb_pad);
> + if (xfs_has_metauuid(mp))
> + return offsetofend(struct xfs_dsb, sb_meta_uuid);
> + if (xfs_has_crc(mp))
> + return offsetofend(struct xfs_dsb, sb_lsn);
> + if (xfs_sb_version_hasmorebits(&mp->m_sb))
> + return offsetofend(struct xfs_dsb, sb_bad_features2);
> + if (xfs_has_logv2(mp))
> + return offsetofend(struct xfs_dsb, sb_logsunit);
> + if (xfs_has_sector(mp))
> + return offsetofend(struct xfs_dsb, sb_logsectsize);
> + /* only support dirv2 or more recent */
> + return offsetofend(struct xfs_dsb, sb_dirblklog);

This really should be libxfs so tht it can be shared with
secondary_sb_whack in xfsrepair. The comment at the end of
the xfs_dsb definition should also be changed to point to this
libxfs version.

> +}
> /* Everything else must be zero. */
> - if (memchr_inv(sb + 1, 0,
> - BBTOB(bp->b_length) - sizeof(struct xfs_dsb)))
> + sblen = xchk_superblock_ondisk_size(mp);
> + if (memchr_inv((char *)sb + sblen, 0, BBTOB(bp->b_length) - sblen))

This could be simplified to

if (memchr_inv(bp->b_addr + sblen, 0, BBTOB(bp->b_length) - sblen))

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>