From: Taylor Blau <me@ttaylorr.com>
Date: Wed, 11 Oct 2023 14:45:43 GMT
Subject: Re: [PATCH 06/20] commit-graph: check consistency of fanout table
Message-ID: <ZSa1F//QwawkTxvm@nand.local>
In-Reply-To: <20231009210458.GF3282181@coredump.intra.peff.net>

On Mon, Oct 09, 2023 at 05:04:58PM -0400, Jeff King wrote:
> We use bsearch_hash() to look up items in the oid index of a
> commit-graph. It also has a fanout table to reduce the initial range in
> which we'll search. But since the fanout comes from the on-disk file, a
> corrupted or malicious file can cause us to look outside of the
> allocated index memory.

This is all very well written and explained. The patch LGTM.

> ---
> So I actually implemented the bsearch_hash() bounds checks and wrote
> tests for midx and idx files before realizing how they handle this. ;)
> Which makes sense, because the usual outcome for a corrupted idx file is
> for it to say "non-monotonic index", which I have seen lead to user
> confusion. Arguably we should have it say something about "hey, your idx
> file seems to be corrupted, because...". But that can be its own topic.

Yeah, I definitely agree that that is out of scope here, and can be left
as #leftoverbits.

Thanks,
Taylor