xfs
[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: avoid firstused overflow in attr3 leaf header with 64k blocks
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 24 Feb 2015 08:53:00 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1424722050-24149-1-git-send-email-bfoster@xxxxxxxxxx>
References: <1424722050-24149-1-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>