* [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
* [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 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
* 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
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).