Thread (4 messages) 4 messages, 3 authors, 2021-02-24

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

flat view

From: Junio C Hamano <hidden>
Date: 2021-02-24 20:10:16

Possibly related (same subject, not in this thread)

"Jeff Hostetler via GitGitGadget" [off-list ref] writes:
From: Jeff Hostetler <redacted>

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 <redacted>
---
    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.
quoted hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help