On Tue, Aug 25, 2020 at 03:18:42PM -0700, Darrick J. Wong wrote: > On Wed, Aug 26, 2020 at 12:10:48AM +0300, Alexey Dobriyan wrote: > > xfs_attr_shortform_verify() contains the following code: > > > > > > int64_t size = ifp->if_bytes; > > /* > > * Give up if the attribute is way too short. > > */ > > if (size < sizeof(struct xfs_attr_sf_hdr)) > > return __this_address; > > > > > > In general "if (signed < sizeof())" is wrong because of how type > > promotions work. Such check won't catch small negative values. > > > > I don't know XFS well enough to know if negative values were excluded > > somewhere above the callchain, but maybe someone else does. > > The initial allocations are always positive and the subsequent > xfs_idata_realloc are checked to prevent if_bytes from going negative, > but it does seem funny to me that if_bytes is declared int64_t... Yes, the int64_t is weird. IIRC the signednes was done to simplify some arithmertics in the reallocation case, but that shouldn't really leak out..
