[PATCH] xfs: avoid firstused overflow in attr3 leaf header with 64k blocks

[Dave Chinner]

On Mon, Feb 23, 2015 at 03:07:30PM -0500, Brian Foster wrote:
> The attr3 leaf header has a 16-bit firstused field that tracks the first
> used entry offset. This field is initialized to the block size in
> xfs_attr3_leaf_create() and updated accordingly in
> xfs_attr3_leaf_add_work() when new attributes are added.
> 
> The initialization of firstused overflows if the block size exceeds
> 16-bits. E.g., xfstests test generic/117 causes assert failures on a
> -bsize=64k fs on ppc64 because ichdr.firstused evaluates to 0.
> 
> Update the firstused initialization to not exceed the maximum value of
> an unsigned short. This avoids the overflow to 0 and allows firstused to
> be updated appropriately on subsequent xattr addition. Also update the
> freemap size calculation to use the actual block size rather than the
> potentially minimized version stored in firstused.
> 
> Signed-off-by: Brian Foster <bfoster at redhat.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 15105db..dc7bda3 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -970,7 +970,8 @@ xfs_attr3_leaf_create(
>  	memset(leaf, 0, args->geo->blksize);
>  
>  	memset(&ichdr, 0, sizeof(ichdr));
> -	ichdr.firstused = args->geo->blksize;
> +	/* firstused is 16-bit */
> +	ichdr.firstused = min_t(int, USHRT_MAX, args->geo->blksize);

Needs a better comment.

>  
>  	if (xfs_sb_version_hascrc(&mp->m_sb)) {
>  		struct xfs_da3_blkinfo *hdr3 = bp->b_addr;
> @@ -986,7 +987,7 @@ xfs_attr3_leaf_create(
>  		ichdr.magic = XFS_ATTR_LEAF_MAGIC;
>  		ichdr.freemap[0].base = sizeof(struct xfs_attr_leaf_hdr);
>  	}
> -	ichdr.freemap[0].size = ichdr.firstused - ichdr.freemap[0].base;
> +	ichdr.freemap[0].size = args->geo->blksize - ichdr.freemap[0].base;

And that also needs an explanation, too.

Cheers,

Dave.
-- 
Dave Chinner
david at fromorbit.com