Re: [PATCH 2/2] packfile: fix corruption due to stale delta base cache entries

> Hi Patrick, 
> 
> > Le 2 oct. 2026 à 03:34, Patrick Steinhardt <ps@pks.im> a écrit :
> > 
> > The delta base cache is a process-global hashmap that is keyed by the
> > address of the `struct packed_git` plus the offset of the base object
> > within that pack. Entries part of the cache are never removed when a
> > pack is closed, and neither when the pack is subsequently freed. As a
> > consequence, the cache may contain stale entries.
> > 
> > For a long time, the worst consequence of this leaking cache was that we
> > held on to memory that we could've released. But the reason for this was
> > that we didn't even free the packfiles, either. That has changed in
> > 6f1e9394e2 (object: fix leaking packfiles when closing object store,
> > 2024-08-08), where we plugged that leak.
> > 
> > Now that we free them, a new packfile may be allocated using the exact
> > same address as a previously allocated one. And if the new packfile has
> > both the same address and a similar layout, it may happen that a
> > preexisting entry from a previously-allocated in the delta base cache
> > would have the exact same key.
> > 
> > All of this sounds very theoretical, but we can actually trigger this
> > bug somewhat reliably! When doing a merge with "--recurse-submodules" in
> > a repository with lots of submodules that have similar-looking packfiles
> 
> merge does not have a --recurse-submodules flag, submodules are merged
> by default (but not updated after the merge, which would be what the
> flag would do if it existed :))

> > When using glibc, one of the packfiles will eventually get the exact
> > same address, and that will then cause Git to read the wrong entry from
> > the cache. Git detects this and aborts with an error:
> > 
> >    $ git merge branch-b
> >    error: Could not read 584ef938be4a749bfa13f68d5ac5545bc029e529
> >    error: could not parse commit 584ef938be4a749bfa13f68d5ac5545bc029e529
> >    error: failed to merge submodule G (repository corrupt)
> > 
> > Now in this case we're lucky that Git detects this error because we try
> > to read a commit from a different submodule via an object database that
> > doesn't have it. But potentially, in an even more contrived scenario, we
> > might even silently yield wrong data from the cache.
> > 
> > Fix this bug by evicting cache entries that belong to a specific pack
> > when closing it.
> > 
> > Note that the added test reliably reproduces the above bug on my machine
> > that uses NixOS at c59305bab206 (cosmic-applets: add missing runtime
> > dependency (#566040), 2026-10-01) with glibc 2.44-25. But as we rely on
> > specific allocation behaviour of glibc it is very likely that the test
> > will not work on other platforms.
> > 
> > Reported-by: Guillaume Chauvel <guillaume.chauvel@gmail.com>
> > Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> Thanks for the trailer and the quick fix !!
> I’m still puzzled why it worked correctly on 2.56.0-rc1 on my WSL
> instance. From your commit message, I guess for some reason I get
> different adresses and so the bug does not trigger.