git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] blame: fix segfault on untracked files

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Aug 29, 2016, 19:50 UTC
Message-ID
<20160829195019.30876-1-t.gummerer@gmail.com>
In-Reply-To
<xmqqeg57bfcu.fsf@gitster.mtv.corp.google.com>

Since 3b75ee9 ("blame: allow to blame paths freshly added to the index", 2016-07-16) git blame also looks at the index to determine if there is a file that was freshly added to the index.

cache_name_pos returns -pos - 1 in case there is no match is found, or if the name matches, but the entry has a stage other than 0. As git blame should work for unmerged files, it uses strcmp to determine whether the name of the returned position matches, in which case the file exists, but is merely unmerged, or if the file actually doesn't exist in the index.

If the repository is empty, or if the file would lexicographically be sorted as the last file in the repository, -cache_name_pos - 1 is outside of the length of the active_cache array, causing git blame to segfault. Guard against that, and die() normally to restore the old behaviour.

Reported-by: Simon Ruderich <simon@ruderich.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Show 9 quoted lines
> The point of this fix is not that we show the exact error message,
> but we fail in a controlled manner.  I think
>
> test_expect_success 'blame untracked file in empty repo' '
>                >untracked &&
>                test_must_fail git blame untracked
>        '
>
> is sufficient.
Yeah, I agree with that.
Thanks for the review!
 builtin/blame.c  | 3 ++-
 t/t8002-blame.sh | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 7ec7823..a5bbf91 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2244,7 +2244,8 @@ static void verify_working_tree_path(struct commit *work_tree, const char *path)
 	pos = cache_name_pos(path, strlen(path));
 	if (pos >= 0)
 		; /* path is in the index */
-	else if (!strcmp(active_cache[-1 - pos]->name, path))
+	else if (-1 - pos < active_nr &&
+		 !strcmp(active_cache[-1 - pos]->name, path))
 		; /* path is in the index, unmerged */
 	else
 		die("no such path '%s' in HEAD", path);
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index ff09ace..ab79de9 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -6,6 +6,11 @@ test_description='git blame'
 PROG='git blame -c'
 . "$TEST_DIRECTORY"/annotate-tests.sh
 
+test_expect_success 'blame untracked file in empty repo' '
+	>untracked &&
+	test_must_fail git blame untracked
+'
+
 PROG='git blame -c -e'
 test_expect_success 'blame --show-email' '
 	check_count \
-- 
2.10.0.rc2.246.g4fd2c60
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 5 in “Crash when using git blame on untracked file”
  1. Simon RuderichAug 27, 2016
  2. Thomas GummererAug 27, 2016
  3. Junio C HamanoAug 29, 2016
  4. blame: fix segfault on untracked filesThomas Gummerer, Aug 29, 2016
  5. Junio C HamanoAug 29, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.