From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 05 Oct 2026 05:32:18 GMT
Subject: Re: [PATCH 2/2] packfile: fix corruption due to stale delta base cache entries
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:
> 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