From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
Jacob Keller <jacob.e.keller@intel.com>,
git@vger.kernel.org
Subject: Re: [PATCH 3.5/4] object-file: fix mmap() leak in odb_source_loose_read_object_stream()
Date: Tue, 10 Mar 2026 13:23:53 +0100 [thread overview]
Message-ID: <abANWS_j2g-ae89b@pks.im> (raw)
In-Reply-To: <xmqqv7f8td6b.fsf@gitster.g>
On Fri, Mar 06, 2026 at 09:35:08PM -0800, Junio C Hamano wrote:
> 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
next prev parent reply other threads:[~2026-03-10 12:23 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-05 20:51 memory leak when cloning a repository Jacob Keller
2026-03-05 22:02 ` Jeff King
2026-03-05 23:03 ` [PATCH 0/4] plugging some mmap() leaks Jeff King
2026-03-05 23:08 ` [PATCH 1/4] check_connected(): delay opening new_pack Jeff King
2026-03-05 23:18 ` Jacob Keller
2026-03-05 23:09 ` [PATCH 2/4] check_connected(): fix leak of pack-index mmap Jeff King
2026-03-05 23:20 ` Jacob Keller
2026-03-05 23:12 ` [PATCH 3/4] pack-revindex: avoid double-loading .rev files Jeff King
2026-03-05 23:13 ` [PATCH 4/4] Makefile: turn on NO_MMAP when building with LSan Jeff King
2026-03-06 9:17 ` Jacob Keller
2026-03-06 16:25 ` [PATCH 5/4] meson: " Jeff King
2026-03-06 18:00 ` Ramsay Jones
2026-03-07 1:14 ` [PATCH 4/4] Makefile: " Junio C Hamano
2026-03-07 2:24 ` [PATCH 3.5/4] object-file: fix mmap() leak in odb_source_loose_read_object_stream() Jeff King
2026-03-07 5:35 ` Junio C Hamano
2026-03-10 12:23 ` Patrick Steinhardt [this message]
2026-03-06 4:37 ` [PATCH 0/4] plugging some mmap() leaks Ramsay Jones
2026-03-06 16:21 ` Jeff King
2026-03-06 17:49 ` Ramsay Jones
2026-03-06 18:37 ` Junio C Hamano
2026-03-06 18:55 ` Ramsay Jones
2026-03-06 22:05 ` Junio C Hamano
2026-03-06 23:25 ` Ramsay Jones
2026-03-07 1:15 ` Junio C Hamano
2026-03-05 23:16 ` memory leak when cloning a repository Jacob Keller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abANWS_j2g-ae89b@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jacob.e.keller@intel.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox