Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter

From: Jeff King <peff@peff.net>
Date: Oct 29, 2012, 06:05 UTC
Message-ID: <20121029060524.GB4457@sigill.intra.peff.net>
In-Reply-To: <508D8DF7.7040007@arcor.de>

On Sun, Oct 28, 2012 at 08:56:39PM +0100, Peter Oberndorfer wrote:

> > The patch below should fix it. I added tests, but please try your
> > real-world test case on it to double-check.
> 
> I tested your patch, but now it crashes for another reason :-)

Well, that's progress, right? :)

Show 8 quoted lines

> i have a file with exactly 12288(0x3000) bytes in the repository.
> When the file is loaded, the data is placed luckily so the data end
> falls at a page boundary.
> Later diff_grep() calls regexec() which calls strlen() on the loaded buffer
> and ends up reading beyond the actual data into the next page
> which is not allocated and causes a pagefault.
> Or it could possibly (randomly) match the regex on data that is not
> actually part of a file...

Yuck. For the most part, we treat blob content (and generally most object content) as a sized buffer. However, there are some spots which, either through laziness or because a code interface expects a string, we pass the value as a string. This works because the object-reading code puts an extra NUL at the end of our buffer to handle just such an instance. So we might prematurely end if the object contains embedded NULs, but we would never read past the end.

The code to read the output of a textconv filter does not do this explicitly. I would think it would get it for free by virtue of reading into a strbuf, though. I'll try to investigate.

-Peff