On Wed, Jul 14, 2021 at 02:18:57PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> The verifier checks explicitly for bp->b_bn == XFS_SB_DADDR to match
> the primary superblock buffer, but the primary superblock is an
> uncached buffer and so bp->b_bn is always -1ULL. Hence this never
> matches and the CRC error reporting is wholly dependent on the
> mount superblock already being populated so CRC feature checks pass
> and allow CRC errors to be reported.
>
> Fix this so that the primary superblock CRC error reporting is not
> dependent on already having read the superblock into memory.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_sb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 04f5386446db..4a4586bd2ba2 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -636,7 +636,7 @@ xfs_sb_read_verify(
>
> if (!xfs_buf_verify_cksum(bp, XFS_SB_CRC_OFF)) {
> /* Only fail bad secondaries on a known V5 filesystem */
> - if (bp->b_bn == XFS_SB_DADDR ||
> + if (bp->b_maps[0].bm_bn == XFS_SB_DADDR ||
I did not know that b_bn only applies to cached buffers.
Would you mind ... I dunno, updating the comment in the struct xfs_buf
declaration to make this clearer?
/*
* Block number of buffer, when this buffer is cached. For
* uncached buffers, only the buffer map (i.e. b_maps[0].bm_bn)
* contains the block number.
*/
xfs_daddr_t b_bn;
With that changed, this looks reasonable to me.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> xfs_sb_version_hascrc(&mp->m_sb)) {
> error = -EFSBADCRC;
> goto out_error;
> --
> 2.31.1
>
