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

[PATCH v4 0/3] Fix a segfault caused by regexec() being called on mmap()ed data

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 21, 2016, 18:23 UTC
Message-ID
<cover.1474482164.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1473321437.git.johannes.schindelin@gmx.de>

[Cc:ing Benjamin Kramer & René Scharfe because they both worked on the REG_STARTEND code in grep.c that I replace in this iteration of the patch series]

This patch series addresses a problem where `git diff` is called using `-G` or `-S --pickaxe-regex` on new-born files that are configured without user diff drivers, and that hence get mmap()ed into memory.

The problem with that: mmap()ed memory is *not* NUL-terminated, yet the pickaxe code calls regexec() on it just the same.

This problem has been reported by my colleague Chris Sidi.

We solve this by introducing a helper, regexec_buf(), that takes a pointer and a length instead of a NUL-terminated string.

This helper then uses REG_STARTEND where available, and falls back to allocating and constructing a NUL-terminated string. Given the wide-spread support for REG_STARTEND (Linux has it, MacOSX has it, Git for Windows has it because it uses compat/regex/ that has it), I think this is a fair trade-off.

Changes since v3:
- reworded the onelines as per Junio's suggestions.
- removed fallback when REG_STARTEND is not supported, in favor of
  requiring NO_REGEX.
- removed the regmatch() function from grep.c, in favor of using
  regexec_buf().
Johannes Schindelin (3):
  regex: -G<pattern> feeds a non NUL-terminated string to regexec() and
    fails
  regex: add regexec_buf() that can work on a non NUL-terminated string
  regex: use regexec_buf()
 Makefile                |  3 ++-
 diff.c                  |  3 ++-
 diffcore-pickaxe.c      | 18 ++++++++----------
 git-compat-util.h       | 13 +++++++++++++
 grep.c                  | 14 ++------------
 t/t4061-diff-pickaxe.sh | 22 ++++++++++++++++++++++
 xdiff-interface.c       | 13 ++++---------
 7 files changed, 53 insertions(+), 33 deletions(-)
 create mode 100755 t/t4061-diff-pickaxe.sh
Published-As: https://github.com/dscho/git/releases/tag/mmap-regexec-v4
Fetch-It-Via: git fetch https://github.com/dscho/git mmap-regexec-v4
Interdiff vs v3:
 diff --git a/Makefile b/Makefile
 index df4f86b..c6f7f66 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -301,7 +301,8 @@ all::
  # crashes due to allocation and free working on different 'heaps'.
  # It's defined automatically if USE_NED_ALLOCATOR is set.
  #
 -# Define NO_REGEX if you have no or inferior regex support in your C library.
 +# Define NO_REGEX if your C library lacks regex support with REG_STARTEND
 +# feature.
  #
  # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
  # user.
 diff --git a/git-compat-util.h b/git-compat-util.h
 index 627ec5f..8aab0c3 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -977,25 +977,17 @@ void git_qsort(void *base, size_t nmemb, size_t size,
  #define qsort git_qsort
  #endif
  
 +#ifndef REG_STARTEND
 +#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
 +#endif
 +
  static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,
  			      size_t nmatch, regmatch_t pmatch[], int eflags)
  {
 -#ifdef REG_STARTEND
  	assert(nmatch > 0 && pmatch);
  	pmatch[0].rm_so = 0;
  	pmatch[0].rm_eo = size;
  	return regexec(preg, buf, nmatch, pmatch, eflags | REG_STARTEND);
 -#else
 -	char *buf2 = xmalloc(size + 1);
 -	int ret;
 -
 -	memcpy(buf2, buf, size);
 -	buf2[size] = '\0';
 -	ret = regexec(preg, buf2, nmatch, pmatch, eflags);
 -	free(buf2);
 -
 -	return ret;
 -#endif
  }
  
  #ifndef DIR_HAS_BSD_GROUP_SEMANTICS
 diff --git a/grep.c b/grep.c
 index d7d00b8..1194d35 100644
 --- a/grep.c
 +++ b/grep.c
 @@ -898,17 +898,6 @@ static int fixmatch(struct grep_pat *p, char *line, char *eol,
  	}
  }
  
 -static int regmatch(const regex_t *preg, char *line, char *eol,
 -		    regmatch_t *match, int eflags)
 -{
 -#ifdef REG_STARTEND
 -	match->rm_so = 0;
 -	match->rm_eo = eol - line;
 -	eflags |= REG_STARTEND;
 -#endif
 -	return regexec(preg, line, 1, match, eflags);
 -}
 -
  static int patmatch(struct grep_pat *p, char *line, char *eol,
  		    regmatch_t *match, int eflags)
  {
 @@ -919,7 +908,8 @@ static int patmatch(struct grep_pat *p, char *line, char *eol,
  	else if (p->pcre_regexp)
  		hit = !pcrematch(p, line, eol, match, eflags);
  	else
 -		hit = !regmatch(&p->regexp, line, eol, match, eflags);
 +		hit = !regexec_buf(&p->regexp, line, eol - line, 1, match,
 +				   eflags);
  
  	return hit;
  }
-- 
2.10.0.windows.1.10.g803177d

base-commit: f6727b0509ec3417a5183ba6e658143275a734f5
Previous: Junio C HamanoNext: Johannes Schindelin
Message 56 of 66 in “Fix a segfault caused by regexec() being called on mmap()ed data”
  1. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 5, 2016
  2. 2/3 diff_populate_filespec: NUL-terminate buffersJohannes Schindelin, Sep 5, 2016
  3. Jeff KingSep 6, 2016
  4. Johannes SchindelinSep 6, 2016
  5. Jeff KingSep 6, 2016
  6. Junio C HamanoSep 7, 2016
  7. Johannes SchindelinSep 8, 2016
  8. Johannes SchindelinSep 8, 2016
  9. Jeff KingSep 8, 2016
  10. Junio C HamanoSep 8, 2016
  11. Johannes SchindelinSep 8, 2016
  12. Jeff KingSep 8, 2016
  13. 3/3 diff_grep: add assertions verifying that the buffers are NUL-terminatedJohannes Schindelin, Sep 5, 2016
  14. Jeff KingSep 6, 2016
  15. Johannes SchindelinSep 6, 2016
  16. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 5, 2016
  17. Jeff KingSep 6, 2016
  18. Johannes SchindelinSep 8, 2016
  19. Junio C HamanoSep 5, 2016
  20. Jeff KingSep 6, 2016
  21. Johannes SchindelinSep 6, 2016
  22. Jeff KingSep 6, 2016
  23. Johannes SchindelinSep 8, 2016
  24. Jeff KingSep 8, 2016
  25. Johannes SchindelinSep 9, 2016
  26. Junio C HamanoSep 9, 2016
  27. Johannes SchindelinSep 6, 2016
  28. Jeff KingSep 6, 2016
  29. Johannes SchindelinSep 6, 2016
  30. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 8, 2016
  31. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  32. 2/3 Introduce a function to run regexec() on non-NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  33. Jeff KingSep 8, 2016
  34. Johannes SchindelinSep 9, 2016
  35. Jeff KingSep 9, 2016
  36. 3/3 Use the newly-introduced regexec_buf() functionJohannes Schindelin, Sep 8, 2016
  37. Johannes SchindelinSep 8, 2016
  38. Jeff KingSep 8, 2016
  39. Jeff KingSep 8, 2016
  40. Jeff KingSep 8, 2016
  41. Ramsay JonesSep 8, 2016
  42. Jeff KingSep 8, 2016
  43. Junio C HamanoSep 8, 2016
  44. Johannes SchindelinSep 8, 2016
  45. Jeff KingSep 8, 2016
  46. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 8, 2016
  47. 1/3 Demonstrate a problem: our pickaxe code assumes NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  48. 2/3 Introduce a function to run regexec() on non-NUL-terminated buffersJohannes Schindelin, Sep 8, 2016
  49. Junio C HamanoSep 8, 2016
  50. 3/3 Use the newly-introduced regexec_buf() functionJohannes Schindelin, Sep 8, 2016
  51. Junio C HamanoSep 8, 2016
  52. Johannes SchindelinSep 9, 2016
  53. Jeff KingSep 9, 2016
  54. Johannes SchindelinSep 9, 2016
  55. Junio C HamanoSep 9, 2016
  56. 0/3 Fix a segfault caused by regexec() being called on mmap()ed dataJohannes Schindelin, Sep 21, 2016
  57. 1/3 regex: -G<pattern> feeds a non NUL-terminated string to regexec() and failsJohannes Schindelin, Sep 21, 2016
  58. 2/3 regex: add regexec_buf() that can work on a non NUL-terminated stringJohannes Schindelin, Sep 21, 2016
  59. Junio C HamanoSep 21, 2016
  60. Johannes SchindelinSep 22, 2016
  61. 3/3 regex: use regexec_buf()Johannes Schindelin, Sep 21, 2016
  62. Junio C HamanoSep 21, 2016
  63. Junio C HamanoSep 21, 2016
  64. Jeff KingSep 21, 2016
  65. Johannes SchindelinSep 25, 2016
  66. Jeff KingSep 21, 2016

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.