git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "André Laszlo" <andre@laszlo.nu>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	"René Scharfe" <l.s.r@web.de>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Jeff King" <peff@peff.net>, "André Laszlo" <andre@laszlo.nu>
Subject: [PATCH] pull: do not segfault when HEAD refers to missing object file
Date: Mon,  6 Mar 2017 00:42:22 +0100	[thread overview]
Message-ID: <20170305234222.4590-1-andre@laszlo.nu> (raw)

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@laszlo.nu>
---

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


             reply	other threads:[~2017-03-05 23:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-05 23:42 André Laszlo [this message]
2017-03-05 23:52 ` [PATCH] pull: do not segfault when HEAD refers to missing object file brian m. carlson
2017-03-06  3:51   ` Jeff King
2017-03-06  6:52     ` Johannes Sixt
2017-03-06  7:33       ` Jeff King
2017-03-06  3:46 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170305234222.4590-1-andre@laszlo.nu \
    --to=andre@laszlo.nu \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=l.s.r@web.de \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).