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

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 5, 2026, 05:32 UTC
Message-ID: <asM2YoImN8bHLHj8@pks.im>
In-Reply-To: <20261002222335.GC833115@coredump.intra.peff.net>

On Fri, Oct 02, 2026 at 06:23:35PM -0400, Jeff King wrote:

Show 12 quoted lines

> On Fri, Oct 02, 2026 at 09:34:07AM +0200, Patrick Steinhardt wrote:
> 
> > 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.
> 
> At its core this is a user-after-free bug, isn't it? If so, I think it
> would be fine to say that ASan will reliably find it (and we don't even
> really need to demonstrate the complex case where the packed_git has the
> same address; all bets are off once we access the freed pointer).

It doesn't though. The key of the cache is the address of the freed object, but the value is a still-live object:

	struct delta_base_cache_key {
		struct packed_git *p;
		off_t base_offset;
	};
	
	struct delta_base_cache_entry {
		struct hashmap_entry ent;
		struct delta_base_cache_key key;
		struct list_head lru;
		void *data;
		size_t size;
		enum object_type type;
	};

We only use the value of `p`, but never dereference it. In fact, when I enable ASan I cannot reproduce the bug at all anymore because it will hand out unique addresses.

Patrick