On Mon, Feb 23, 2015 at 02:32:29PM -0600, Eric Sandeen wrote:
> 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;
And a loop in xfs_attr3_leaf_remove() that does:
tmp = args->geo->blksize;
.....
ichdr.firstused = tmp;
so if the the loop in between does not modify tmp...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|