Re: [PATCH] builtin/gc.c: fix use-after-free in maintenance_unregister()

[Taylor Blau <me@ttaylorr.com>]

On Tue, Nov 15, 2022 at 08:41:44PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > I am disappointed in myself for finding this only after I pushed out a
> > hotfix to 'next' and rebuild the downstream branches.
> >
> > This should be a minimal fix on top of Ævar's patch to get 'next'
> > building again.
>
> I'm also disappointed in myself, sorry. I *did* test it locally with
> valgrind, but obviously fat-fingered it somehow and tested the wrong
> version. Sorry!

It's OK. Let's not beat ourselves up too much, and instead focusing on
making sure the quality for the next pushout is higher (which is more on
me than it is on you).

> >  builtin/gc.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index d87cf84041..38882a1e35 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1543,6 +1543,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
> >  	int found = 0;
> >  	struct string_list_item *item;
> >  	const struct string_list *list;
> > +	struct config_set cs = { { 0 } };
>
> Just "{ 0 }" here instead? I see it may have been copied from some older
> pre-image though, and they'll do the same in either case, so it's not
> important...

Copying from other zero-initializations of `struct config_set`:

    $ git grep -oh 'struct config_set.*= {.*' | sort | uniq -c
          3 struct config_set cs = { { 0 } };

Thanks,
Taylor