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

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Emily Shaffer <emilyshaffer@google.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH] unpack-trees: watch for out-of-range index position
Date: Thu, 9 Jan 2020 02:52:50 -0500	[thread overview]
Message-ID: <20200109075250.GA3978837@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqo8vd1yb2.fsf@gitster-ct.c.googlers.com>

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

> >> 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