git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH] cache-tree: avoid infinite loop on zero-entry tree

From
Jeff King <peff@peff.net>
Date
Oct 29, 2014, 18:52 UTC
Message-ID
<20141029185249.GA26740@peff.net>
In-Reply-To
<xmqqppdayal7.fsf@gitster.dls.corp.google.com>
On Wed, Oct 29, 2014 at 11:50:12AM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > I'm a little iffy on this just because it is fixing one particular bug,
> > and I am sure there are probably a bunch of other ways to have a bogus
> > index. Fundamentally, I think we pretty much trust that the index was
> > not maliciously generated (unlike packfiles, for instance, which can
> > come from elsewhere).  Still, this is one step closer to safe, and the
> > bug was seen in the wild, so maybe it is worth doing.
> 
> Is it cheap to sanity-check the input when we map in the cache-tree
> upon read_cache()?  Then we can just invalidate the cache-tree,
> either in its entirety (easy) or just the bogus subpart (maybe not
> worth doing).
I think it is not super-expensive, but it is not as easy as:
  if (!it->entry_count)
	return -1;
Show 12 quoted lines
> > We could alternatively (or in addition) reject 0-entry cache trees when
> > reading them from disk. The trick, though, is that it is not just
> > records with 0 entries, but ones where the sum of the entries and
> > subtree entries is 0. Given that it is not something we expect to
> > happen, it is easier to catch it here. And we know there can be no
> > regressions for missed corner cases, because the case we are catching
> > here would _always_ have gone into an infinite loop before this patch.
> 
> OK.  I wonder if we can instead die here but propagate the error
> back up the callchain and have the ultimate caller rebuild the cache
> tree without paying attention to the existing data that we now know
> is bogus.

Yeah, that would make sense to me, but I was not familiar with the cache-tree code to do it easily (and given that this is not something that should ever happen, I didn't want to spend time digging in).

I can provide you with a real-world test case if you want to explore it further.

-Peff
Previous: Junio C Hamano
Message 3 of 3 in “cache-tree: avoid infinite loop on zero-entry tree”
  1. cache-tree: avoid infinite loop on zero-entry treeJeff King, Oct 29, 2014
  2. Junio C HamanoOct 29, 2014
  3. Jeff KingOct 29, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.