Re: [PATCH 06/11] pack-redundant: avoid using uninitialized memory
From: Junio C Hamano <hidden>
Date: 2022-06-16 04:53:09
"Johannes Schindelin via GitGitGadget" [off-list ref] writes:
quoted hunk
From: Johannes Schindelin <redacted> In the `add_pack()` function, we forgot to initialize the field `next`, which could potentially lead to readin uninitialized memory later. Reported by Coverity. Signed-off-by: Johannes Schindelin <redacted> --- builtin/pack-redundant.c | 1 + 1 file changed, 1 insertion(+)diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index ed9b9013a5f..1f7da1f68b6 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c@@ -526,6 +526,7 @@ static struct pack_list * add_pack(struct packed_git *p) } l.all_objects_size = l.remaining_objects->size; l.unique_objects = NULL; + l.next = NULL; if (p->pack_local) return pack_list_insert(&local_packs, &l); else
return pack_list_insert(&altodb_packs, &l);
The pack_list_insert reads like so:
static inline struct pack_list * pack_list_insert(struct pack_list **pl,
struct pack_list *entry)
{
struct pack_list *p = xmalloc(sizeof(struct pack_list));
memcpy(p, entry, sizeof(struct pack_list));
p->next = *pl;
*pl = p;
return p;
}
IOW, we prepare members of "l", pass it as "entry" to pack_list_insert(),
but the helper function allocates a new piece of memory, copies "l"
to it, and then sets the .next member of it to a reasonable value.
And after that, the "l" that was merely used as a template goes out
of scope.
So I think this is a false positive. memcpy(p) does copy the
uninitialized garbage that is held in entry->next to p->next, but it
is overwritten with a reasonable address immediately after that.