patchdir: fix malloc of root untracked_cache_dir

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> 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.

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> 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.
>
> A problem was observed on Windows where the length of the memset() was
> too short, so the first byte of the name field was not zeroed.  This
> resulted in the name field having garbage from a previous use of that
> area of memory.
>
> The record for the root directory was then written to the untracked-cache
> extension in the index.  This garbage would then be visible to future
> commands when they reloaded the untracked-cache extension.
>
> Since the directory record for the root directory had garbage in the
> `name` field, the `t/helper/test-tool dump-untracked-cache` tool
> printed this garbage as the path prefix (rather than '/') for each
> directory in the untracked cache as it recursed.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>     dir: fix malloc of root untracked_cache_dir

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-884%2Fjeffhostetler%2Funtracked-cache-corruption-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-884/jeffhostetler/untracked-cache-corruption-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/884
>
>  dir.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index d153a63bbd14..fd8aa7c40faa 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2730,11 +2730,8 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
>  		return NULL;
>  	}
>  
> -	if (!dir->untracked->root) {
> -		const int len = sizeof(*dir->untracked->root);
> -		dir->untracked->root = xmalloc(len);
> -		memset(dir->untracked->root, 0, len);
> -	}
> +	if (!dir->untracked->root)
> +		FLEX_ALLOC_STR(dir->untracked->root, name, "");
>  
>  	/* Validate $GIT_DIR/info/exclude and core.excludesfile */
>  	root = dir->untracked->root;
>
> base-commit: 966e671106b2fd38301e7c344c754fd118d0bb07

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

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

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