"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> 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 <johannes.schindelin@xxxxxx>
> ---
> 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.
