Re: [PATCH 3.5/4] object-file: fix mmap() leak in odb_source_loose_read_object_stream()
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> 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.
Show 28 quoted lines
>
> You can see the leak in the test suite by building with:
>
> make SANITIZE=leak NO_MMAP=1 CC=clang
>
> and running t1060. We need NO_MMAP so that the mmap() is backed by an
> actual malloc(), which allows LSan to detect it. And the leak seems not
> to be detected when compiling with gcc, probably due to some internal
> compiler decisions about how the stack memory is written.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> object-file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/object-file.c b/object-file.c
> index 3094140055..ab2fb9c4eb 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2197,7 +2197,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> return 0;
> error:
> git_inflate_end(&st->z);
> - munmap(st->mapped, st->mapsize);
> + munmap(mapped, mapsize);
> free(st);
> return -1;
> }