Re: [PATCH v2 1/6] odb: introduce interface to generate packfiles

On Sun, Aug 16, 2026 at 10:40 PM Patrick Steinhardt <[email protected]> wrote:
>
> +static int odb_source_files_generate_pack(struct odb_source *source UNUSED,
> +                                         struct odb_pack_generator **out,
> +                                         const struct odb_generate_pack_options *opts)
> +{
> +       struct child_process cp = CHILD_PROCESS_INIT;
> +       struct odb_pack_generator_files *generator;
> +       FILE *in;
[...]
> +       cp.clean_on_exit = 1;
> +
> +       if (start_command(&cp))
> +               return error(_("could not spawn pack-objects"));
[...]
> +       CALLOC_ARRAY(generator, 1);
> +       generator->base.out = opts->pack_fd < 0 ? cp.out : -1;
> +       generator->base.err = opts->progress_fd < 0 ? cp.err : -1;
> +       generator->base.finish = odb_pack_generator_files_finish;
> +       generator->cp = cp;
> +
> +       *out = &generator->base;
> +       return 0;
> +}

Does this have a use-after-scope bug lurking here, due to the
combination of clean_on_exit = 1 (which makes a copy of &cp for later
use), and the fact that cp is a function-local?  If I'm reading the
code right, start_command() calls mark_child_for_cleanup(), which does

    p->process = process;  /* where process is &cp */

and then cleanup_children() accesses various fields under p->process.
You do copy the necessary fields from cp to generator->cp, but
&generator->cp was not passed to start_command(), so p->process points
to the function-local cp.

I think the normal teardown path happens to be fine despite this
issue: when odb_pack_generator_files_finish() calls
finish_command(&generator->cp), it clears the child by matching pid
(which was copied separately from p->process), so the stale pointer
never gets dereferenced in the successful path.  But with an
abnormal-exit, which is where clean_on_exit comes into play, then
cleanup_children() will be called and start attempting to read
p->process, which now points to some long-reclaimed function stack
space.