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

[Junio C Hamano <gitster@pobox.com>]

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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

Nicely spotted.

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.

Will queue; thanks.

> 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