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

[PATCH v4 1/3] regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails

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

When our pickaxe code feeds file contents to regexec(), it implicitly assumes that the file contents are read into implicitly NUL-terminated buffers (i.e. that we overallocate by 1, appending a single '\0').

This is not so.

In particular when the file contents are simply mmap()ed, we can be virtually certain that the buffer is preceding uninitialized bytes, or invalid pages.

Note that the test we add here is known to be flakey: we simply cannot know whether the byte following the mmap()ed ones is a NUL or not.

Typically, on Linux the test passes. On Windows, it fails virtually every time due to an access violation (that's a segmentation fault for you Unix-y people out there). And Windows would be correct: the regexec() call wants to operate on a regular, NUL-terminated string, there is no NUL in the mmap()ed memory range, and it is undefined whether the next byte is even legal to access.

When run with --valgrind it demonstrates quite clearly the breakage, of course.

Being marked with `test_expect_failure`, this test will sometimes be declare "TODO fixed", even if it only passes by mistake.

This test case represents a Minimal, Complete and Verifiable Example of a breakage reported by Chris Sidi.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t4061-diff-pickaxe.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100755 t/t4061-diff-pickaxe.sh
diff --git a/t/t4061-diff-pickaxe.sh b/t/t4061-diff-pickaxe.sh
new file mode 100755
index 0000000..5929f2e
--- /dev/null
+++ b/t/t4061-diff-pickaxe.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Johannes Schindelin
+#
+
+test_description='Pickaxe options'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_commit initial &&
+	printf "%04096d" 0 >4096-zeroes.txt &&
+	git add 4096-zeroes.txt &&
+	test_tick &&
+	git commit -m "A 4k file"
+'
+test_expect_failure '-G matches' '
+	git diff --name-only -G "^0{4096}$" HEAD^ >out &&
+	test 4096-zeroes.txt = "$(cat out)"
+'
+
+test_done
-- 
2.10.0.windows.1.10.g803177d
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 57 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.