On Wed, Aug 26, 2020 at 10:39:26AM -0500, Eric Sandeen wrote: > On 8/26/20 10:13 AM, Darrick J. Wong wrote: > > ... > > > TBH I think this ought to be fixed by changing the declaration of > > xfs_attr_sf_entry.nameval to "uint8_t nameval[]" and using more modern > > fugly macros like struct_sizeof() to calculate the entry sizes without > > us all having to remember to subtract one from the struct size. > > Fair, but I think that in the interest of time we should fix it up with a -1 > which is consistent with the other bits of attr code first, then this can all > be cleaned up by making it a [] not [1], dropping the magical -1, turning > the macros into functions ala dir2, etc. > > Sound ok? Yes. sorry, I thought I was suggesting that we start with the quick -1 fix and move on to fixing the struct, but ENOCOFFEE and LPC sessions start too early... :( --d > >> No. I should do that, good point. Now I do wonder if > >> > >> /* > >> * Check that the variable-length part of the structure is > >> * within the data buffer. The next entry starts after the > >> * name component, so nextentry is an acceptable test. > >> */ > >> next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep); > >> if ((char *)next_sfep > endp) > >> return __this_address; > >> > >> should be >= but I'll have to unravel all the macros to see. In that case > >> though the missing "=" makes it too lenient not too strict, at least. > > > > *endp points to the first byte after the end of the buffer, because it > > is defined as (*sfp + size). The end of the last *sfep in the sf attr > > struct is supposed to coincide with the end of the buffer, so changing > > this to >= is not correct. > > Let me think on that a little more ;) > > -Eric
