Re: [PATCH] dir: fix malloc of root untracked_cache_dir

[Jeff Hostetler <git@jeffhostetler.com>]

On 2/24/21 4:05 PM, Jeff King wrote:
> On Wed, Feb 24, 2021 at 12:08:42PM -0800, Junio C Hamano wrote:
> 
>>> Use FLEX_ALLOC_STR() to allocate the `struct untracked_cache_dir`
>>> for the root directory.  Get rid of unsafe code that might fail to
>>> initialize the `name` field (if FLEX_ARRAY is not 1).  This will
>>> make it clear that we intend to have a structure with an empty
>>> string following it.
>> [...]
>> The problematic code was introduced in 2015, a year before these
>> FLEX_ALLOC_*() helpers were introduced.  The result is of course
>> correct and much easier to read, as the necessary "ask for a region
>> of calloc'ed memory with an additional byte for terminating NUL
>> beyond strlen()" is hidden in the helper.
> 
> When I added the FLEX_ALLOC_* helpers, I audited for existing callers to
> convert. But I did so by looking for places where we were doing manual
> size computations. The bug here was that it was not doing any
> computation at all (when it need to be doing "+1"). So that's my guess
> why it got overlooked (which is not super important, but may give a hint
> about how to look for similar bugs).


There's another call to xmalloc() at [1] that does the st_add3()
thing that I didn't change.  It was correctly computing the size
so I didn't want to change it for no reason.  That and the 2 memcpy()s
made it feel like we should have a different FLEX_ macro for this
case -- more of a FLEX_DUP... or something.  But I didn't want to
distract from my bug fix here.

[1] https://github.com/git/git/blob/master/dir.c#L3290

Jeff