{"thread":{"id":"45596","subject":"[PATCH v2 0/2] name-hash: fix buffer overrun","startedAt":"2017-04-03T15:17:07Z","lastAt":"2017-04-13T06:27:07Z","messageCount":5,"participants":["git@jeffhostetler.com","Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"316090","messageId":"20170403151642.2889-1-git@jeffhostetler.com","threadId":"45596","inReplyTo":null,"subject":"[PATCH v2 0/2] name-hash: fix buffer overrun","fromName":"","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-03T15:16:40Z","receivedAt":"2017-04-03T15:17:07Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nVersion 2 of this fix smashes the HT and chmod issues in the\ntest code discussed on the mailing list. The test has also\nbeen updated to skip on 1 cpu systems.\n\n================\nFix buffer overrun in handle_range_dir() when the final entry\nin the index was the only file in the last directory, such as\n\"a/b/foo.txt\". The look ahead (k_start + 1) was invalid since\n(k_start + 1) == k_end.\n\nJeff Hostetler (1):\n test-online-cpus: helper to return cpu count\n\nKevin Willford (1):\n name-hash: fix buffer overrun\n\n Makefile | 1 +\n name-hash.c | 4 +++-\n t/helper/.gitignore | 1 +\n t/helper/test-online-cpus.c | 8 ++++++++\n t/t3008-ls-files-lazy-init-name-hash.sh | 27 +++++++++++++++++++++++++++\n 5 files changed, 40 insertions(+), 1 deletion(-)\n create mode 100644 t/helper/test-online-cpus.c\n create mode 100755 t/t3008-ls-files-lazy-init-name-hash.sh\n\n-- \n2.9.3\n\n"},{"id":"316091","messageId":"20170403151642.2889-2-git@jeffhostetler.com","threadId":"45596","inReplyTo":"20170403151642.2889-1-git@jeffhostetler.com","subject":"[PATCH v2 1/2] test-online-cpus: helper to return cpu count","fromName":"","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-03T15:16:41Z","receivedAt":"2017-04-03T15:17:09Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreated helper executable to print the value of online_cpus()\nallowing multi-threaded tests to be skipped when appropriate.\n\nSigned-off-by: Jeff Hostetler \n---\n Makefile | 1 +\n t/helper/.gitignore | 1 +\n t/helper/test-online-cpus.c | 8 ++++++++\n 3 files changed, 10 insertions(+)\n create mode 100644 t/helper/test-online-cpus.c\n\ndiff --git a/Makefile b/Makefile\nindex 9b36068..3bb31e9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -626,6 +626,7 @@ TEST_PROGRAMS_NEED_X += test-line-buffer\n TEST_PROGRAMS_NEED_X += test-match-trees\n TEST_PROGRAMS_NEED_X += test-mergesort\n TEST_PROGRAMS_NEED_X += test-mktemp\n+TEST_PROGRAMS_NEED_X += test-online-cpus\n TEST_PROGRAMS_NEED_X += test-parse-options\n TEST_PROGRAMS_NEED_X += test-path-utils\n TEST_PROGRAMS_NEED_X += test-prio-queue\ndiff --git a/t/helper/.gitignore b/t/helper/.gitignore\nindex 758ed2e..b05d67c 100644\n--- a/t/helper/.gitignore\n+++ b/t/helper/.gitignore\n@@ -16,6 +16,7 @@\n /test-match-trees\n /test-mergesort\n /test-mktemp\n+/test-online-cpus\n /test-parse-options\n /test-path-utils\n /test-prio-queue\ndiff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c\nnew file mode 100644\nindex 0000000..c881073\n--- /dev/null\n+++ b/t/helper/test-online-cpus.c\n@@ -0,0 +1,8 @@\n+#include \"stdio.h\"\n+#include \"thread-utils.h\"\n+\n+int cmd_main(int argc, const char **argv)\n+{\n+\tprintf(\"%d\\n\", online_cpus());\n+\treturn 0;\n+}\n-- \n2.9.3\n\n"},{"id":"316092","messageId":"20170403151642.2889-3-git@jeffhostetler.com","threadId":"45596","inReplyTo":"20170403151642.2889-1-git@jeffhostetler.com","subject":"[PATCH v2 2/2] name-hash: fix buffer overrun","fromName":"","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-03T15:16:42Z","receivedAt":"2017-04-03T15:17:11Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Kevin Willford \n\nAdd check for the end of the entries for the thread partition.\nAdd test for lazy init name hash with specific directory structure\n\nThe lazy init hash name was causing a buffer overflow when the last\nentry in the index was multiple folder deep with parent folders that\ndid not have any files in them.\n\nThis adds a test for the boundary condition of the thread partitions\nwith the folder structure that was triggering the buffer overflow.\nThe test is skipped on single-cpu machines because the original code\npath is used in name-hash.c\n\nThe fix was to check if it is the last entry for the thread partition\nin the handle_range_dir and not try to use the next entry in the cache.\n\nSigned-off-by: Kevin Willford \nSigned-off-by: Johannes Schindelin \nSigned-off-by: Jeff Hostetler \n---\n name-hash.c | 4 +++-\n t/t3008-ls-files-lazy-init-name-hash.sh | 27 +++++++++++++++++++++++++++\n 2 files changed, 30 insertions(+), 1 deletion(-)\n create mode 100755 t/t3008-ls-files-lazy-init-name-hash.sh\n\ndiff --git a/name-hash.c b/name-hash.c\nindex cac313c..39309ef 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -342,7 +342,9 @@ static int handle_range_dir(\n \t * Scan forward in the index array for index entries having the same\n \t * path prefix (that are also in this directory).\n \t */\n-\tif (strncmp(istate->cache[k_start + 1]->name, prefix->buf, prefix->len) > 0)\n+\tif (k_start + 1 >= k_end)\n+\t\tk = k_end;\n+\telse if (strncmp(istate->cache[k_start + 1]->name, prefix->buf, prefix->len) > 0)\n \t\tk = k_start + 1;\n \telse if (strncmp(istate->cache[k_end - 1]->name, prefix->buf, prefix->len) == 0)\n \t\tk = k_end;\ndiff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh\nnew file mode 100755\nindex 0000000..bdf5198\n--- /dev/null\n+++ b/t/t3008-ls-files-lazy-init-name-hash.sh\n@@ -0,0 +1,27 @@\n+#!/bin/sh\n+\n+test_description='Test the lazy init name hash with various folder structures'\n+\n+. ./test-lib.sh\n+\n+if test 1 -eq $($GIT_BUILD_DIR/t/helper/test-online-cpus)\n+then\n+\tskip_all='skipping lazy-init tests, single cpu'\n+\ttest_done\n+fi\n+\n+LAZY_THREAD_COST=2000\n+\n+test_expect_success 'no buffer overflow in lazy_init_name_hash' '\n+\t(\n+\t test_seq $LAZY_THREAD_COST | sed \"s/^/a_/\"\n+\t echo b/b/b\n+\t test_seq $LAZY_THREAD_COST | sed \"s/^/c_/\"\n+\t test_seq 50 | sed \"s/^/d_/\" | tr \"\\n\" \"/\"; echo d\n+\t) |\n+\tsed \"s/^/100644 $EMPTY_BLOB\t/\" |\n+\tgit update-index --index-info &&\n+\ttest-lazy-init-name-hash -m\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"316160","messageId":"45b6c815-bae4-e33e-f4d6-172fdc7de7e2@ramsayjones.plus.com","threadId":"45596","inReplyTo":"20170403151642.2889-2-git@jeffhostetler.com","subject":"Re: [PATCH v2 1/2] test-online-cpus: helper to return cpu count","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-04-04T16:38:47Z","receivedAt":"2017-04-04T16:38:59Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 03/04/17 16:16, git@jeffhostetler.com wrote:\n> From: Jeff Hostetler \n> \n> Created helper executable to print the value of online_cpus()\n> allowing multi-threaded tests to be skipped when appropriate.\n> \n> Signed-off-by: Jeff Hostetler \n> ---\n> Makefile | 1 +\n> t/helper/.gitignore | 1 +\n> t/helper/test-online-cpus.c | 8 ++++++++\n> 3 files changed, 10 insertions(+)\n> create mode 100644 t/helper/test-online-cpus.c\n> \n> diff --git a/Makefile b/Makefile\n> index 9b36068..3bb31e9 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -626,6 +626,7 @@ TEST_PROGRAMS_NEED_X += test-line-buffer\n> TEST_PROGRAMS_NEED_X += test-match-trees\n> TEST_PROGRAMS_NEED_X += test-mergesort\n> TEST_PROGRAMS_NEED_X += test-mktemp\n> +TEST_PROGRAMS_NEED_X += test-online-cpus\n> TEST_PROGRAMS_NEED_X += test-parse-options\n> TEST_PROGRAMS_NEED_X += test-path-utils\n> TEST_PROGRAMS_NEED_X += test-prio-queue\n> diff --git a/t/helper/.gitignore b/t/helper/.gitignore\n> index 758ed2e..b05d67c 100644\n> --- a/t/helper/.gitignore\n> +++ b/t/helper/.gitignore\n> @@ -16,6 +16,7 @@\n> /test-match-trees\n> /test-mergesort\n> /test-mktemp\n> +/test-online-cpus\n> /test-parse-options\n> /test-path-utils\n> /test-prio-queue\n> diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c\n> new file mode 100644\n> index 0000000..c881073\n> --- /dev/null\n> +++ b/t/helper/test-online-cpus.c\n> @@ -0,0 +1,8 @@\n> +#include \"stdio.h\"\n> +#include \"thread-utils.h\"\n> +\n> +int cmd_main(int argc, const char **argv)\n> +{\n> +\tprintf(\"%d\\n\", online_cpus());\n> +\treturn 0;\n> +}\n> \n\nIn order to suppress a warning (for lack of extern declaration of\ncmd_main), we need to include git-compat-util.h ( or cache.h etc,.),\nlike so:\n\n $ git diff\n diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c\n index c88107360..b5277fb50 100644\n --- a/t/helper/test-online-cpus.c\n +++ b/t/helper/test-online-cpus.c\n @@ -1,3 +1,4 @@\n +#include \"git-compat-util.h\"\n #include \"stdio.h\"\n #include \"thread-utils.h\"\n \n $ \n\nOtherwise, this series fixes the test for me.\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n"},{"id":"316713","messageId":"xmqqfuhcq0ul.fsf@gitster.mtv.corp.google.com","threadId":"45596","inReplyTo":"20170403151642.2889-3-git@jeffhostetler.com","subject":"Re: [PATCH v2 2/2] name-hash: fix buffer overrun","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-13T06:26:58Z","receivedAt":"2017-04-13T06:27:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"git@jeffhostetler.com writes:\n\n> From: Kevin Willford \n>\n> Add check for the end of the entries for the thread partition.\n> Add test for lazy init name hash with specific directory structure\n>\n> The lazy init hash name was causing a buffer overflow when the last\n> entry in the index was multiple folder deep with parent folders that\n> did not have any files in them.\n>\n> This adds a test for the boundary condition of the thread partitions\n> with the folder structure that was triggering the buffer overflow.\n> The test is skipped on single-cpu machines because the original code\n> path is used in name-hash.c\n>\n> The fix was to check if it is the last entry for the thread partition\n> in the handle_range_dir and not try to use the next entry in the cache.\n\nAs I merged the older one already to 'next', I'll queue 1/2 of v2 on\ntop of them and then the following (which is incremental between v1\nand this v2 2/2) on top.\n\n-- >8 --\nFrom: Kevin Willford \nDate: Mon, 3 Apr 2017 15:16:42 +0000\nSubject: [PATCH] t3008: skip lazy-init test on a single-core box\n\nThe lazy-init codepath will not be exercised uniless threaded. Skip\nthe entire test on a single-core box. Also replace a hard-coded\nconstant of 2000 (number of cache entries to manifacture for tests)\nwith a variable with a human readable name.\n\nSigned-off-by: Kevin Willford \nSigned-off-by: Johannes Schindelin \nSigned-off-by: Jeff Hostetler \nSigned-off-by: Junio C Hamano \n---\n t/t3008-ls-files-lazy-init-name-hash.sh | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh\nindex 971975bff4..bdf5198b7e 100755\n--- a/t/t3008-ls-files-lazy-init-name-hash.sh\n+++ b/t/t3008-ls-files-lazy-init-name-hash.sh\n@@ -4,14 +4,22 @@ test_description='Test the lazy init name hash with various folder structures'\n \n . ./test-lib.sh\n \n+if test 1 -eq $($GIT_BUILD_DIR/t/helper/test-online-cpus)\n+then\n+\tskip_all='skipping lazy-init tests, single cpu'\n+\ttest_done\n+fi\n+\n+LAZY_THREAD_COST=2000\n+\n test_expect_success 'no buffer overflow in lazy_init_name_hash' '\n \t(\n-\t test_seq 2000 | sed \"s/^/a_/\"\n+\t test_seq $LAZY_THREAD_COST | sed \"s/^/a_/\"\n \t echo b/b/b\n-\t test_seq 2000 | sed \"s/^/c_/\"\n+\t test_seq $LAZY_THREAD_COST | sed \"s/^/c_/\"\n \t test_seq 50 | sed \"s/^/d_/\" | tr \"\\n\" \"/\"; echo d\n \t) |\n-\tsed -e \"s/^/100644 $EMPTY_BLOB\t/\" |\n+\tsed \"s/^/100644 $EMPTY_BLOB\t/\" |\n \tgit update-index --index-info &&\n \ttest-lazy-init-name-hash -m\n '\n-- \n2.12.2-776-gded3dc243c\n\n"}]}