Re: [PATCH] xfs: fix boundary test in xfs_attr_shortform_verify

[Eric Sandeen <sandeen@sandeen.net>]

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?

>> 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