All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.