From: Jacob Keller Date: Thu, 05 Mar 2026 23:20:11 GMT Subject: Re: [PATCH 2/4] check_connected(): fix leak of pack-index mmap Message-ID: In-Reply-To: <20260305230956.GB2901305@coredump.intra.peff.net> On Thu, Mar 5, 2026 at 3:10 PM Jeff King wrote: > > 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 > --- Reviewed-by: Jacob Keller