[PATCH 04/32] xfs: check magic numbers in dir3 leaf verifier first

[Eric Sandeen]

On 9/29/13 10:15 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner at redhat.com>
> 
> Calling xfs_dir3_leaf_hdr_from_disk() in a verifier before
> validating the magic numbers in the buffer results in ASSERT
> failures due to mismatching magic numbers when a corruption occurs.
> Seeing as the verifier is supposed to catch the corruption and pass
> it back to the caller, having the verifier assert fail on error
> defeats the purpose of detecting the errors in the first place.
> 
> Check the magic numbers direct from the buffer before decoding the
> header.

Looks good; have you sent this for the kernel yet?

(I thought we wanted changes to hit kernelspace first) :)

Reviewed-by: Eric Sandeen <sandeen at redhat.com>

> Signed-off-by: Dave Chinner <dchinner at redhat.com>
> ---
>  libxfs/xfs_dir2_leaf.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/libxfs/xfs_dir2_leaf.c b/libxfs/xfs_dir2_leaf.c
> index 7ec2f19..c035c4d 100644
> --- a/libxfs/xfs_dir2_leaf.c
> +++ b/libxfs/xfs_dir2_leaf.c
> @@ -161,6 +161,11 @@ xfs_dir3_leaf_check_int(
>  	return true;
>  }
>  
> +/*
> + * We verify the magic numbers before decoding the leaf header so that on debug
> + * kernels we don't get assertion failures in xfs_dir3_leaf_hdr_from_disk() due
> + * to incorrect magic numbers.
> + */
>  static bool
>  xfs_dir3_leaf_verify(
>  	struct xfs_buf		*bp,
> @@ -172,24 +177,25 @@ xfs_dir3_leaf_verify(
>  
>  	ASSERT(magic == XFS_DIR2_LEAF1_MAGIC || magic == XFS_DIR2_LEAFN_MAGIC);
>  
> -	xfs_dir3_leaf_hdr_from_disk(&leafhdr, leaf);
>  	if (xfs_sb_version_hascrc(&mp->m_sb)) {
>  		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
> +		__uint16_t		magic3;
>  
> -		if ((magic == XFS_DIR2_LEAF1_MAGIC &&
> -		     leafhdr.magic != XFS_DIR3_LEAF1_MAGIC) ||
> -		    (magic == XFS_DIR2_LEAFN_MAGIC &&
> -		     leafhdr.magic != XFS_DIR3_LEAFN_MAGIC))
> -			return false;
> +		magic3 = (magic == XFS_DIR2_LEAF1_MAGIC) ? XFS_DIR3_LEAF1_MAGIC
> +							 : XFS_DIR3_LEAFN_MAGIC;
>  
> +		if (leaf3->info.hdr.magic != cpu_to_be16(magic3))
> +			return false;
>  		if (!uuid_equal(&leaf3->info.uuid, &mp->m_sb.sb_uuid))
>  			return false;
>  		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
>  			return false;
>  	} else {
> -		if (leafhdr.magic != magic)
> +		if (leaf->hdr.info.magic != cpu_to_be16(magic))
>  			return false;
>  	}
> +
> +	xfs_dir3_leaf_hdr_from_disk(&leafhdr, leaf);
>  	return xfs_dir3_leaf_check_int(mp, &leafhdr, leaf);
>  }
>  
>