* [PATCH v2 0/2] name-hash: fix buffer overrun @ 2017-04-03 15:16 git 2017-04-03 15:16 ` [PATCH v2 1/2] test-online-cpus: helper to return cpu count git 2017-04-03 15:16 ` [PATCH v2 2/2] name-hash: fix buffer overrun git 0 siblings, 2 replies; 5+ messages in thread From: git @ 2017-04-03 15:16 UTC (permalink / raw) To: git; +Cc: gitster, peff, ramsay, Jeff Hostetler From: Jeff Hostetler <jeffhost@microsoft.com> 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 -- 2.9.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] test-online-cpus: helper to return cpu count 2017-04-03 15:16 [PATCH v2 0/2] name-hash: fix buffer overrun git @ 2017-04-03 15:16 ` git 2017-04-04 16:38 ` Ramsay Jones 2017-04-03 15:16 ` [PATCH v2 2/2] name-hash: fix buffer overrun git 1 sibling, 1 reply; 5+ messages in thread From: git @ 2017-04-03 15:16 UTC (permalink / raw) To: git; +Cc: gitster, peff, ramsay, Jeff Hostetler From: Jeff Hostetler <jeffhost@microsoft.com> Created helper executable to print the value of online_cpus() allowing multi-threaded tests to be skipped when appropriate. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> --- 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 diff --git a/Makefile b/Makefile index 9b36068..3bb31e9 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/t/helper/.gitignore b/t/helper/.gitignore index 758ed2e..b05d67c 100644 --- a/t/helper/.gitignore +++ b/t/helper/.gitignore @@ -16,6 +16,7 @@ /test-match-trees /test-mergesort /test-mktemp +/test-online-cpus /test-parse-options /test-path-utils /test-prio-queue diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c new file mode 100644 index 0000000..c881073 --- /dev/null +++ b/t/helper/test-online-cpus.c @@ -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; +} -- 2.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] test-online-cpus: helper to return cpu count 2017-04-03 15:16 ` [PATCH v2 1/2] test-online-cpus: helper to return cpu count git @ 2017-04-04 16:38 ` Ramsay Jones 0 siblings, 0 replies; 5+ messages in thread From: Ramsay Jones @ 2017-04-04 16:38 UTC (permalink / raw) To: git, git; +Cc: gitster, peff, Jeff Hostetler On 03/04/17 16:16, git@jeffhostetler.com wrote: > From: Jeff Hostetler <jeffhost@microsoft.com> > > Created helper executable to print the value of online_cpus() > allowing multi-threaded tests to be skipped when appropriate. > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > --- > 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 > > diff --git a/Makefile b/Makefile > index 9b36068..3bb31e9 100644 > --- a/Makefile > +++ b/Makefile > @@ -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 > diff --git a/t/helper/.gitignore b/t/helper/.gitignore > index 758ed2e..b05d67c 100644 > --- a/t/helper/.gitignore > +++ b/t/helper/.gitignore > @@ -16,6 +16,7 @@ > /test-match-trees > /test-mergesort > /test-mktemp > +/test-online-cpus > /test-parse-options > /test-path-utils > /test-prio-queue > diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c > new file mode 100644 > index 0000000..c881073 > --- /dev/null > +++ b/t/helper/test-online-cpus.c > @@ -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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] name-hash: fix buffer overrun 2017-04-03 15:16 [PATCH v2 0/2] name-hash: fix buffer overrun git 2017-04-03 15:16 ` [PATCH v2 1/2] test-online-cpus: helper to return cpu count git @ 2017-04-03 15:16 ` git 2017-04-13 6:26 ` Junio C Hamano 1 sibling, 1 reply; 5+ messages in thread From: git @ 2017-04-03 15:16 UTC (permalink / raw) To: git Cc: gitster, peff, ramsay, Kevin Willford, Johannes Schindelin, Jeff Hostetler From: Kevin Willford <kewillf@microsoft.com> 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. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> --- 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 diff --git a/name-hash.c b/name-hash.c index cac313c..39309ef 100644 --- a/name-hash.c +++ b/name-hash.c @@ -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; diff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh new file mode 100755 index 0000000..bdf5198 --- /dev/null +++ b/t/t3008-ls-files-lazy-init-name-hash.sh @@ -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 -- 2.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] name-hash: fix buffer overrun 2017-04-03 15:16 ` [PATCH v2 2/2] name-hash: fix buffer overrun git @ 2017-04-13 6:26 ` Junio C Hamano 0 siblings, 0 replies; 5+ messages in thread From: Junio C Hamano @ 2017-04-13 6:26 UTC (permalink / raw) To: git Cc: git, peff, ramsay, Kevin Willford, Johannes Schindelin, Jeff Hostetler git@jeffhostetler.com writes: > From: Kevin Willford <kewillf@microsoft.com> > > 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. 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 <kewillf@microsoft.com> 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. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t3008-ls-files-lazy-init-name-hash.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh index 971975bff4..bdf5198b7e 100755 --- a/t/t3008-ls-files-lazy-init-name-hash.sh +++ b/t/t3008-ls-files-lazy-init-name-hash.sh @@ -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 ' -- 2.12.2-776-gded3dc243c ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-13 6:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-03 15:16 [PATCH v2 0/2] name-hash: fix buffer overrun git 2017-04-03 15:16 ` [PATCH v2 1/2] test-online-cpus: helper to return cpu count git 2017-04-04 16:38 ` Ramsay Jones 2017-04-03 15:16 ` [PATCH v2 2/2] name-hash: fix buffer overrun git 2017-04-13 6:26 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.