From db5dfa331480650c1f889db3cb32a0272dc72ec6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 21 Sep 2016 20:23:22 +0200 Subject: regex: -G feeds a non NUL-terminated string to regexec() and fails 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 Signed-off-by: Junio C Hamano --- t/t4062-diff-pickaxe.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100755 t/t4062-diff-pickaxe.sh (limited to 't') diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh new file mode 100755 index 0000000000..5929f2eabb --- /dev/null +++ b/t/t4062-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 -- cgit v1.2.3 From b7d36ffca02c23f545d6e098d78180e6e72dfd8d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 21 Sep 2016 20:24:14 +0200 Subject: regex: use regexec_buf() The new regexec_buf() function operates on buffers with an explicitly specified length, rather than NUL-terminated strings. We need to use this function whenever the buffer we want to pass to regexec(3) may have been mmap(2)ed (and is hence not NUL-terminated). Note: the original motivation for this patch was to fix a bug where `git diff -G ` would crash. This patch converts more callers, though, some of which allocated to construct NUL-terminated strings, or worse, modified buffers to temporarily insert NULs while calling regexec(3). By converting them to use regexec_buf(), the code has become much cleaner. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t4062-diff-pickaxe.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't') diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh index 5929f2eabb..f0bf50bda7 100755 --- a/t/t4062-diff-pickaxe.sh +++ b/t/t4062-diff-pickaxe.sh @@ -14,7 +14,7 @@ test_expect_success setup ' test_tick && git commit -m "A 4k file" ' -test_expect_failure '-G matches' ' +test_expect_success '-G matches' ' git diff --name-only -G "^0{4096}$" HEAD^ >out && test 4096-zeroes.txt = "$(cat out)" ' -- cgit v1.2.3