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

Re: [PATCHv2] pickaxe: fix segfault with '-S<...> --pickaxe-regex'

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Mar 21, 2017, 11:46 UTC
Message-ID
<alpine.DEB.2.20.1703211244470.3767@virtualbox>
In-Reply-To
<20170318182408.4444-1-szeder.dev@gmail.com>
Hi Gábor,
On Sat, 18 Mar 2017, SZEDER Gábor wrote:
Show 29 quoted lines
> 'git {log,diff,...} -S<...> --pickaxe-regex' can segfault as a result of
> out-of-bounds memory reads.
> 
> diffcore-pickaxe.c:contains() looks for all matches of the given regex
> in a buffer in a loop, advancing the buffer pointer to the end of the
> last match in each iteration.  When we switched to REG_STARTEND in
> b7d36ffca (regex: use regexec_buf(), 2016-09-21), we started passing
> the size of that buffer to the regexp engine, too.  Unfortunately,
> this buffer size is never updated on subsequent iterations, and as the
> buffer pointer advances on each iteration, this "bufptr+bufsize"
> points past the end of the buffer.  This results in segmentation
> fault, if that memory can't be accessed.  In case of 'git log' it can
> also result in erroneously listed commits, if the memory past the end
> of buffer is accessible and happens to contain data matching the
> regex.
> 
> Reduce the buffer size on each iteration as the buffer pointer is
> advanced, thus maintaining the correct end of buffer location.
> Furthermore, make sure that the buffer pointer is not dereferenced in
> the control flow statements when we already reached the end of the
> buffer.
> 
> The new test is flaky, I've never seen it fail on my Linux box even
> without the fix, but this is expected according to db5dfa3 (regex:
> -G<pattern> feeds a non NUL-terminated string to regexec() and fails,
> 2016-09-21).  However, it did fail on Travis CI with the first (and
> incomplete) version of the fix, and based on that commit message I
> would expect the new test without the fix to fail most of the time on
> Windows.

Thank you for catching and fixing this. On Windows, I indeed get a segmentation fault for the new test case without the patched code, and the patch indeed fixes this.

ACK, Dscho

Previous: SZEDER GáborNext: Junio C Hamano
Message 4 of 6 in “pickaxe: fix segfault with '-S<...> --pickaxe-regex'”
  1. pickaxe: fix segfault with '-S<...> --pickaxe-regex'SZEDER Gábor, Mar 18, 2017
  2. SZEDER GáborMar 18, 2017
  3. [PATCHv2] pickaxe: fix segfault with '-S<...> --pickaxe-regex'SZEDER Gábor, Mar 18, 2017
  4. Johannes SchindelinMar 21, 2017
  5. Junio C HamanoMar 18, 2017
  6. Junio C HamanoMar 18, 2017

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.