Re: [PATCH 3.5/4] object-file: fix mmap() leak in odb_source_loose_read_object_stream()
On Fri, Mar 06, 2026 at 09:35:08PM -0800, Junio C Hamano wrote:
Show 26 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > Subject: object-file: fix mmap() leak in odb_source_loose_read_object_stream()
> >
> > We mmap() a loose object file, storing the result in the local variable
> > "mapped", which is eventually assigned into our stream struct as
> > "st.mapped". If we hit an error, we jump to an error label which does:
> >
> > munmap(st.mapped, st.mapsize);
> >
> > to clean up. But this is wrong; we don't assign st.mapped until the end
> > of the function, after all of the "goto error" jumps. So this munmap()
> > is never cleaning up anything (st.mapped is always NULL, because we
> > initialize the struct with calloc).
> >
> > Instead, we should feed the local variable to munmap().
> >
> > This leak is due to 595296e124 (streaming: allocate stream inside the
> > backend-specific logic, 2025-11-23), which introduced the local
> > variable. Before that, we assigned the mmap result directly into
> > st.mapped. It was probably switched there so that we do not have to
> > allocate/free the struct when the map operation fails (e.g., because we
> > don't have the loose object). Before that commit, the struct was passed
> > in from the caller, so there was no allocation at all.
>
> Makes sense. Thanks for finding and fixing the issue so quickly.
Yup, indeed, this is an obvious fix. Thanks for cleaning up after me!
Patrick