All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marcel Röthke" <marcel@roethke.info>
To: git@vger.kernel.org
Cc: "Marcel Röthke" <marcel@roethke.info>
Subject: [PATCH v3] rerere: fix crashes due to unmatched opening conflict markers
Date: Tue,  9 Apr 2024 14:13:51 +0200	[thread overview]
Message-ID: <20240409121708.131542-2-marcel@roethke.info> (raw)
In-Reply-To: <20240218194603.1210895-1-marcel@roethke.info>

When rerere handles a conflict with an unmatched opening conflict marker
in a file with other conflicts, it will fail create a preimage and also
fail allocate the status member of struct rerere_dir. Currently the
status member is allocated after the error handling. This will lead to a
SEGFAULT when the status member is accessed during cleanup of the failed
parse.

Additionally, in subsequent executions of rerere, after removing the
MERGE_RR.lock manually, rerere crashes for a similar reason. MERGE_RR
points to a conflict id that has no preimage, therefore the status
member is not allocated and a SEGFAULT happens when trying to check if a
preimage exists.

Solve this by making sure the status field is allocated correctly and add
tests to prevent the bug from reoccurring.

This does not fix the root cause, failing to parse stray conflict
markers, but I don't think we can do much better than recognizing it,
printing an error, and moving on gracefully.

Signed-off-by: Marcel Röthke <marcel@roethke.info>
---
Interdiff against v2:
  diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
  index fb53dddf79..1e80f76860 100755
  --- a/t/t4200-rerere.sh
  +++ b/t/t4200-rerere.sh
  @@ -671,4 +671,67 @@ test_expect_success 'test simple stage 1 handling' '
   	)
   '

  +test_expect_success 'rerere does not crash with missing preimage' '
  +	git config rerere.enabled true &&
  +
  +	echo bar >test &&
  +	git add test &&
  +	git commit -m "one" &&
  +	git branch rerere_no_crash &&
  +
  +	echo foo >>test &&
  +	git add test &&
  +	git commit -m "two" &&
  +
  +	git checkout rerere_no_crash &&
  +	echo "bar" >>test &&
  +	git add test &&
  +	git commit -m "three" &&
  +
  +	test_must_fail git rebase main &&
  +	rm .git/rr-cache/*/preimage &&
  +	git rebase --abort
  +'
  +
  +test_expect_success 'rerere does not crash with unmatched conflict marker' '
  +	git config rerere.enabled true &&
  +
  +	echo bar >test &&
  +	git add test &&
  +	git commit -m "one" &&
  +	git branch rerere_no_preimage &&
  +
  +	cat >test <<-EOF &&
  +	test
  +	bar
  +	foobar
  +	EOF
  +	git add test &&
  +	git commit -m "two" &&
  +
  +	git checkout rerere_no_preimage &&
  +	echo "bar" >>test &&
  +	git add test &&
  +	git commit -m "three" &&
  +
  +	cat >test <<-EOF &&
  +	foobar
  +	bar
  +	bar
  +	EOF
  +	git add test &&
  +	git commit -m "four" &&
  +
  +	test_must_fail git rebase main &&
  +	cat >test <<-EOF &&
  +	test
  +	bar
  +	<<<<<<< HEAD
  +	foobar
  +	bar
  +	EOF
  +	git add test &&
  +	git rebase --continue
  +'
  +
   test_done

 rerere.c          |  5 ++++
 t/t4200-rerere.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/rerere.c b/rerere.c
index ca7e77ba68..4683d6cbb1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -219,6 +219,11 @@ static void read_rr(struct repository *r, struct string_list *rr)
 		buf.buf[hexsz] = '\0';
 		id = new_rerere_id_hex(buf.buf);
 		id->variant = variant;
+		/*
+		 * make sure id->collection->status has enough space
+		 * for the variant we are interested in
+		 */
+		fit_variant(id->collection, variant);
 		string_list_insert(rr, path)->util = id;
 	}
 	strbuf_release(&buf);
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index fb53dddf79..1e80f76860 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -671,4 +671,67 @@ test_expect_success 'test simple stage 1 handling' '
 	)
 '

+test_expect_success 'rerere does not crash with missing preimage' '
+	git config rerere.enabled true &&
+
+	echo bar >test &&
+	git add test &&
+	git commit -m "one" &&
+	git branch rerere_no_crash &&
+
+	echo foo >>test &&
+	git add test &&
+	git commit -m "two" &&
+
+	git checkout rerere_no_crash &&
+	echo "bar" >>test &&
+	git add test &&
+	git commit -m "three" &&
+
+	test_must_fail git rebase main &&
+	rm .git/rr-cache/*/preimage &&
+	git rebase --abort
+'
+
+test_expect_success 'rerere does not crash with unmatched conflict marker' '
+	git config rerere.enabled true &&
+
+	echo bar >test &&
+	git add test &&
+	git commit -m "one" &&
+	git branch rerere_no_preimage &&
+
+	cat >test <<-EOF &&
+	test
+	bar
+	foobar
+	EOF
+	git add test &&
+	git commit -m "two" &&
+
+	git checkout rerere_no_preimage &&
+	echo "bar" >>test &&
+	git add test &&
+	git commit -m "three" &&
+
+	cat >test <<-EOF &&
+	foobar
+	bar
+	bar
+	EOF
+	git add test &&
+	git commit -m "four" &&
+
+	test_must_fail git rebase main &&
+	cat >test <<-EOF &&
+	test
+	bar
+	<<<<<<< HEAD
+	foobar
+	bar
+	EOF
+	git add test &&
+	git rebase --continue
+'
+
 test_done
--
2.44.0


  parent reply	other threads:[~2024-04-09 12:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-18 11:49 [PATCH] rerere: fix crash in during clear Marcel Röthke
2024-02-18 13:02 ` Kristoffer Haugsbakk
2024-02-18 19:38   ` Marcel Röthke
2024-02-18 19:46 ` [PATCH v2] rerere: fix crash " Marcel Röthke
2024-02-20  1:22   ` Junio C Hamano
2024-02-24 11:25     ` Marcel Röthke
2024-03-24 21:51       ` Junio C Hamano
2024-03-25 19:30         ` Marcel Röthke
2024-04-07 20:12         ` Marcel Röthke
2024-04-08 15:18           ` Junio C Hamano
2024-04-09 12:13   ` Marcel Röthke [this message]
2024-04-12 23:37     ` [PATCH v3] rerere: fix crashes due to unmatched opening conflict markers Junio C Hamano
2024-04-15 20:15     ` Junio C Hamano
2024-04-16 10:50       ` Marcel Röthke
2024-04-16 10:52     ` [PATCH v4] " Marcel Röthke

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=20240409121708.131542-2-marcel@roethke.info \
    --to=marcel@roethke.info \
    --cc=git@vger.kernel.org \
    /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.