name-hash: fix buffer overrun
v2 cover name-hash: fix buffer overrun
From: Jeff Hostetler <[email protected]>
Version 2 of this fix smashes the HT and chmod issues in the test code discussed on the mailing list. The test has also been updated to skip on 1 cpu systems.
================
Fix buffer overrun in handle_range_dir() when the final entry in the index was the only file in the last directory, such as "a/b/foo.txt". The look ahead (k_start + 1) was invalid since (k_start + 1) == k_end.
Jeff Hostetler (1):
test-online-cpus: helper to return cpu count
Kevin Willford (1):
name-hash: fix buffer overrun
Makefile | 1 + name-hash.c | 4 +++- t/helper/.gitignore | 1 + t/helper/test-online-cpus.c | 8 ++++++++ t/t3008-ls-files-lazy-init-name-hash.sh | 27 +++++++++++++++++++++++++++ 5 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 t/helper/test-online-cpus.c create mode 100755 t/t3008-ls-files-lazy-init-name-hash.sh
Signature · 1 line
2.9.3
PATCH v2 1/2 test-online-cpus: helper to return cpu count
From: Jeff Hostetler <[email protected]>
Created helper executable to print the value of online_cpus() allowing multi-threaded tests to be skipped when appropriate.
--- Makefile | 1 + t/helper/.gitignore | 1 + t/helper/test-online-cpus.c | 8 ++++++++ 3 files changed, 10 insertions(+) create mode 100644 t/helper/test-online-cpus.c
Makefile +1 -0
@@ -626,6 +626,7 @@ TEST_PROGRAMS_NEED_X += test-line-buffer TEST_PROGRAMS_NEED_X += test-match-trees TEST_PROGRAMS_NEED_X += test-mergesort TEST_PROGRAMS_NEED_X += test-mktemp +TEST_PROGRAMS_NEED_X += test-online-cpus TEST_PROGRAMS_NEED_X += test-parse-options TEST_PROGRAMS_NEED_X += test-path-utils TEST_PROGRAMS_NEED_X += test-prio-queue
t/helper/.gitignore +1 -0
@@ -16,6 +16,7 @@ /test-match-trees /test-mergesort /test-mktemp +/test-online-cpus /test-parse-options /test-path-utils /test-prio-queue
t/helper/test-online-cpus.c new file +8 -0
@@ -0,0 +1,8 @@ +#include "stdio.h" +#include "thread-utils.h" + +int cmd_main (int argc,const char **argv) +{ +printf ("%d\n" ,online_cpus ()); +return 0 ; +}
Signature · 1 line
2.9.3
Re: [PATCH v2 1/2] test-online-cpus: helper to return cpu count
Quoted 44 lines · On 03/04/17 16:16, [email protected] wrote:
From: Jeff Hostetler <[email protected]>
Created helper executable to print the value of
online_cpus()allowing multi-threaded tests to be skipped when appropriate.Signed-off-by Jeff Hostetler <[email protected]>--- Makefile | 1 + t/helper/.gitignore | 1 + t/helper/test-online-cpus.c | 8 ++++++++ 3 files changed, 10 insertions(+) create mode 100644 t/helper/test-online-cpus.cMakefile +1
-0@@ -626,6 +626,7 @@ TEST_PROGRAMS_NEED_X += test-line-buffer TEST_PROGRAMS_NEED_X += test-match-trees TEST_PROGRAMS_NEED_X += test-mergesort TEST_PROGRAMS_NEED_X += test-mktemp +TEST_PROGRAMS_NEED_X += test-online-cpus TEST_PROGRAMS_NEED_X += test-parse-options TEST_PROGRAMS_NEED_X += test-path-utils TEST_PROGRAMS_NEED_X += test-prio-queuet/helper/.gitignore +1
-0@@ -16,6 +16,7 @@ /test-match-trees /test-mergesort /test-mktemp +/test-online-cpus /test-parse-options /test-path-utils /test-prio-queuet/helper/test-online-cpus.c new file +0
-0
t/helper/test-online-cpus.c line 8
Quoted 8 lines
t/helper/test-online-cpus.c new file +8
-0@@ -0,0 +1,8 @@ +#include "stdio.h" +#include "thread-utils.h" + +int cmd_main (int argc,const char **argv) +{ +printf ("%d\n" ,online_cpus ()); +return 0 ; +}
In order to suppress a warning (for lack of extern declaration of cmd_main), we need to include git-compat-util.h ( or cache.h etc,.), like so:
$ git diff diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c index c88107360..b5277fb50 100644 --- a/t/helper/test-online-cpus.c +++ b/t/helper/test-online-cpus.c @@ -1,3 +1,4 @@ +#include "git-compat-util.h" #include "stdio.h" #include "thread-utils.h"
$
Otherwise, this series fixes the test for me.
Thanks!
ATB,
Ramsay Jones
PATCH v2 2/2 name-hash: fix buffer overrun
In reply to [email protected]From: Kevin Willford <[email protected]>
Add check for the end of the entries for the thread partition. Add test for lazy init name hash with specific directory structure
The lazy init hash name was causing a buffer overflow when the last entry in the index was multiple folder deep with parent folders that did not have any files in them.
This adds a test for the boundary condition of the thread partitions with the folder structure that was triggering the buffer overflow. The test is skipped on single-cpu machines because the original code path is used in name-hash.c
The fix was to check if it is the last entry for the thread partition in the handle_range_dir and not try to use the next entry in the cache.
--- name-hash.c | 4 +++- t/t3008-ls-files-lazy-init-name-hash.sh | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100755 t/t3008-ls-files-lazy-init-name-hash.sh
name-hash.c +3 -1
@@ -342,7 +342,9 @@ static int handle_range_dir(* Scan forward in the index array for index entries having the same * path prefix (that are also in this directory). */ -if (strncmp (istate->cache[k_start +1 ]->name, prefix->buf, prefix->len) >0 ) +if (k_start +1 >= k_end) + k = k_end; +else if (strncmp (istate->cache[k_start +1 ]->name, prefix->buf, prefix->len) >0 ) k = k_start +1 ;else if (strncmp (istate->cache[k_end -1 ]->name, prefix->buf, prefix->len) ==0 ) k = k_end;
t/t3008-ls-files-lazy-init-name-hash.sh new file +27 -0
@@ -0,0 +1,27 @@ +#!/bin/sh + +test_description='Test the lazy init name hash with various folder structures' + +. ./test-lib.sh + +if test 1 -eq $($GIT_BUILD_DIR/t/helper/test-online-cpus) +then + skip_all='skipping lazy-init tests, single cpu' + test_done +fi + +LAZY_THREAD_COST=2000 + +test_expect_success 'no buffer overflow in lazy_init_name_hash' ' + ( + test_seq $LAZY_THREAD_COST | sed "s/^/a_/" + echo b/b/b + test_seq $LAZY_THREAD_COST | sed "s/^/c_/" + test_seq 50 | sed "s/^/d_/" | tr "\n" "/"; echo d + ) | + sed "s/^/100644 $EMPTY_BLOB /" | + git update-index --index-info && + test-lazy-init-name-hash -m +' + +test_done
Signature · 1 line
2.9.3
Quoted 9 lines · [email protected] writes:
From: Kevin Willford <[email protected]>
Add check for the end of the entries for the thread partition. Add test for lazy init name hash with specific directory structure
The lazy init hash name was causing a buffer overflow when the last entry in the index was multiple folder deep with parent folders that did not have any files in them.
Quoted 7 lines
This adds a test for the boundary condition of the thread partitions with the folder structure that was triggering the buffer overflow. The test is skipped on single-cpu machines because the original code path is used in
name-hash.cThe fix was to check if it is the last entry for the thread partition in the
handle_range_dirand not try to use the next entry in the cache.
As I merged the older one already to 'next', I'll queue 1/2 of v2 on top of them and then the following (which is incremental between v1 and this v2 2/2) on top.
-- >8 --
From: Kevin Willford <[email protected]>
Date: Mon, 3 Apr 2017 15:16:42 +0000
Subject: [PATCH] t3008: skip lazy-init test on a single-core box
The lazy-init codepath will not be exercised uniless threaded. Skip the entire test on a single-core box. Also replace a hard-coded constant of 2000 (number of cache entries to manifacture for tests) with a variable with a human readable name.
--- t/t3008-ls-files-lazy-init-name-hash.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
t/t3008-ls-files-lazy-init-name-hash.sh +11 -3
@@ -4,14 +4,22 @@ test_description='Test the lazy init name hash with various folder structures' . ./test-lib.sh +if test 1 -eq $($GIT_BUILD_DIR/t/helper/test-online-cpus) +then + skip_all='skipping lazy-init tests, single cpu' + test_done +fi + +LAZY_THREAD_COST=2000 + test_expect_success 'no buffer overflow in lazy_init_name_hash' ' ( - test_seq 2000 | sed "s/^/a_/" + test_seq $LAZY_THREAD_COST | sed "s/^/a_/" echo b/b/b - test_seq 2000 | sed "s/^/c_/" + test_seq $LAZY_THREAD_COST | sed "s/^/c_/" test_seq 50 | sed "s/^/d_/" | tr "\n" "/"; echo d ) | - sed -e "s/^/100644 $EMPTY_BLOB /" | + sed "s/^/100644 $EMPTY_BLOB /" | git update-index --index-info && test-lazy-init-name-hash -m '
Signature · 1 line
2.12.2-776-gded3dc243c