Re: [PATCH 2/4] check_connected(): fix leak of pack-index mmap
On Thu, Mar 5, 2026 at 3:10 PM Jeff King <peff@peff.net> wrote:
Show 26 quoted lines
>
> Since c6807a40dc (clone: open a shortcut for connectivity check,
> 2013-05-26), we may open a one-off packed_git struct to check what's in
> the pack we just received. At the end of the function we throw away the
> struct (rather than linking it into the repository struct as usual).
>
> We used to leak the struct until dd4143e7bf (connected.c: free the
> "struct packed_git", 2022-11-08), which calls free(). But that's not
> sufficient; inside the struct we'll have mmap'd the pack idx data from
> disk, which needs an munmap() call.
>
> Building with SANITIZE=leak doesn't detect this, because we are leaking
> our own mmap(), and it only finds heap allocations from malloc(). But if
> we use our compat mmap implementation like this:
>
> make NO_MMAP=MapsBecomeMallocs SANITIZE=leak
>
> then LSan will notice the leak, because now it's a regular heap buffer
> allocated by malloc().
>
> We can fix it by calling close_pack(), which will free any associated
> memory. Note that we need to check for NULL ourselves; unlike free(), it
> is not safe to pass a NULL pointer to close_pack().
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
Reviewed-by: Jacob Keller <jacob.keller@gmail.com>