Web lists-archives.com

[PATCH] pull: do not segfault when HEAD refers to missing object file




git pull --rebase on a corrupted HEAD used to segfault; it has been
corrected to error out with a message. A test has also been added to
verify this fix.

Signed-off-by: André Laszlo <andre@xxxxxxxxx>
---

Notes:
    When add_head_to_pending fails to add a pending object, git pull
    --rebase segfaults. This can happen if HEAD is referring to a corrupt
    or missing object.
    
    I discovered this segfault on my machine after pulling a repo from
    GitHub, but I have been unable to reproduce the sequence of events
    that lead to the corrupted HEAD (I think it may have been caused by a
    lost network connection in my case).
    
    The following commands use add_head_to_pending:
    
    format-patch  setup_revisions before add_head_to_pending
    diff          checks rev.pending.nr
    shortlog      checks rev.pending.nr
    log           uses resolve_ref_unsafe
    
    All of the above return an error code of 128 and print "fatal: bad
    object HEAD" instead of segfaulting, which I think is correct
    behavior. The check and error message have been added to
    has_uncommitted_changes, where they were missing, as well as to
    diff-lib.c (without the error message).

 diff-lib.c      |  2 +-
 t/t5520-pull.sh | 12 ++++++++++++
 wt-status.c     |  5 +++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/diff-lib.c b/diff-lib.c
index 52447466b..9d26b18c3 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -512,7 +512,7 @@ int run_diff_index(struct rev_info *revs, int cached)
 	struct object_array_entry *ent;
 
 	ent = revs->pending.objects;
-	if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
+	if (!ent || diff_cache(revs, ent->item->oid.hash, ent->name, cached))
 		exit(128);
 
 	diff_set_mnemonic_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 17f4d0fe4..1edb6a97a 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -664,4 +664,16 @@ test_expect_success 'git pull --rebase against local branch' '
 	test file = "$(cat file2)"
 '
 
+test_expect_success 'git pull --rebase with corrupt HEAD does not segfault' '
+	mkdir corrupted &&
+	(cd corrupted &&
+	git init &&
+	echo one >file && git add file &&
+	git commit -m one &&
+	REV=$(git rev-parse HEAD) &&
+	rm -f .git/objects/${REV:0:2}/${REV:2} &&
+	test_expect_code 128 git pull --rebase > /dev/null
+	)
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index d47012048..3d60eaff5 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2252,6 +2252,11 @@ int has_uncommitted_changes(int ignore_submodules)
 		DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
 	DIFF_OPT_SET(&rev_info.diffopt, QUICK);
 	add_head_to_pending(&rev_info);
+
+	/* The add_head_to_pending call might not have added anything. */
+	if (!rev_info.pending.nr)
+		die("bad object %s", "HEAD");
+
 	diff_setup_done(&rev_info.diffopt);
 	result = run_diff_index(&rev_info, 1);
 	return diff_result_code(&rev_info.diffopt, result);
-- 
2.12.0