On 2/23/15 2:07 PM, 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.
cool :)
> 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.
I'm a little scared by this; does this truncated value risk going to disk?
(Yes, I think so.) Is that ok? Does that ... mean we lose a byte of space
we'd otherwise have? Maybe that's ok ...
FWIW, I think the same problem exists in xfs_attr3_leaf_compact():
/* Initialise the incore headers */
ichdr_src = *ichdr_dst; /* struct copy */
ichdr_dst->firstused = args->geo->blksize;
and xfs_attr3_leaf_unbalance():
tmphdr.firstused = state->args->geo->blksize;
> 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);
>
> 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;
But now freemap.size is out of sync with firstused; is that ok?
-Eric
> xfs_attr3_leaf_hdr_to_disk(leaf, &ichdr);
> xfs_trans_log_buf(args->trans, bp, 0, args->geo->blksize - 1);
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
