Re: [RFC PATCH] unpack-trees: watch for out-of-range index position

From: Jeff King <peff@peff.net>
Date: Jan 9, 2020, 07:52 UTC
Message-ID: <20200109075250.GA3978837@coredump.intra.peff.net>
In-Reply-To: <xmqqo8vd1yb2.fsf@gitster-ct.c.googlers.com>

On Wed, Jan 08, 2020 at 12:35:29PM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> >> It does not sound like a BUG to me, either, but the new condition
> >> does look correct to me, too.  We can turn it into die() later if
> >> somebody truly cares ;-)
> >> 
> >> Thanks, both.  Will queue.
> >
> > Thanks much for the quick turnaround. If I hear more noise I'll give it
> > a try with die() or error code instead, but for now I'll move on to the
> > next bug on my list. :)
> 
> By the way, it is somewhat sad that we proceeded that far in the
> first place---such a corrupt on-disk index would have caused an
> early die() if we did not get rid of the trailing-hash integrity
> check.

Perhaps. The integrity check only protects against an index that was modified after the fact, not one that was generated by a buggy Git. I'm not sure we know how the index that led to this patch got into this state (though it sounds like Emily has a copy and could check the hash on it), but other cache-tree segfault I found recently was with an index with an intact integrity hash.

So I think regardless of the trailing-hash check, we'd always want to be defensive when reading on-disk data.

-Peff