[PATCH] rerere: fix crash in during clear

[PATCH] rerere: fix crash in during clear

[Marcel Röthke]

When rerere_clear is called, for instance when aborting a rebase, and
the current conflict does not have a pre or postimage recorded git
crashes with a SEGFAULT in has_rerere_resolution when accessing the
status member of struct rerere_dir. This happens because scan_rerere_dir
only allocates the status field in struct rerere_dir when a post or
preimage was found. In some cases a segfault may happen even if a post
or preimage was recorded if it was not for the variant of interest and
the number of the variant that is present is lower than the variant of
interest.

This patch solves this by making sure the status field is large enough
to accommodate for the variant of interest so it can be accesses without
checking if it is large enough.

An alternative solution would be to always check before accessing the
status field, but I think the chosen solution aligns better with the
assumptions made elsewhere in the code.

Signed-off-by: Marcel Röthke <marcel@roethke.info>
---
 rerere.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rerere.c b/rerere.c
index ca7e77ba68..3973ccce37 100644
--- a/rerere.c
+++ b/rerere.c
@@ -219,6 +219,9 @@ 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);
-- 
2.43.2

Re: [PATCH] rerere: fix crash in during clear

[Kristoffer Haugsbakk]

Hi

> rerere: fix crash in during clear

“in during clear”? Did you mean “during clear”?

On Sun, Feb 18, 2024, at 12:49, Marcel Röthke wrote:
> When rerere_clear is called, for instance when aborting a rebase, and
> the current conflict does not have a pre or postimage recorded git
> crashes with a SEGFAULT in has_rerere_resolution when accessing the
> status member of struct rerere_dir. This happens because scan_rerere_dir
> only allocates the status field in struct rerere_dir when a post or
> preimage was found. In some cases a segfault may happen even if a post
> or preimage was recorded if it was not for the variant of interest and
> the number of the variant that is present is lower than the variant of
> interest.
>
> This patch solves this by making sure the status field is large enough

You can simplify “This patch solves this” to “Solve this”; see
`SubmittingPatches` under “imperative-mood”.

> to accommodate for the variant of interest so it can be accesses without
> checking if it is large enough.

“accessed”

>
> An alternative solution would be to always check before accessing the
> status field, but I think the chosen solution aligns better with the
> assumptions made elsewhere in the code.
>
> Signed-off-by: Marcel Röthke <marcel@roethke.info>
> ---
>  rerere.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/rerere.c b/rerere.c
> index ca7e77ba68..3973ccce37 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -219,6 +219,9 @@ 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 */

Multi-line comments should have the delimiters on separate lines from
the text. See `CodingGuidelines` under “Multi-line comments”.

> +		fit_variant(id->collection, variant);
>  		string_list_insert(rr, path)->util = id;
>  	}
>  	strbuf_release(&buf);
> --
> 2.43.2

Re: [PATCH] rerere: fix crash in during clear

[Marcel Röthke]

On 2024-02-18 14:02:42, Kristoffer Haugsbakk wrote:
> > rerere: fix crash in during clear
>
> “in during clear”? Did you mean “during clear”?

Yes, thanks.

> On Sun, Feb 18, 2024, at 12:49, Marcel Röthke wrote:
> > When rerere_clear is called, for instance when aborting a rebase, and
> > the current conflict does not have a pre or postimage recorded git
> > crashes with a SEGFAULT in has_rerere_resolution when accessing the
> > status member of struct rerere_dir. This happens because scan_rerere_dir
> > only allocates the status field in struct rerere_dir when a post or
> > preimage was found. In some cases a segfault may happen even if a post
> > or preimage was recorded if it was not for the variant of interest and
> > the number of the variant that is present is lower than the variant of
> > interest.
> >
> > This patch solves this by making sure the status field is large enough
>
> You can simplify “This patch solves this” to “Solve this”; see
> `SubmittingPatches` under “imperative-mood”.

done

>
> > to accommodate for the variant of interest so it can be accesses without
> > checking if it is large enough.
>
> “accessed”
>

done

> >
> > An alternative solution would be to always check before accessing the
> > status field, but I think the chosen solution aligns better with the
> > assumptions made elsewhere in the code.
> >
> > Signed-off-by: Marcel Röthke <marcel@roethke.info>
> > ---
> >  rerere.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/rerere.c b/rerere.c
> > index ca7e77ba68..3973ccce37 100644
> > --- a/rerere.c
> > +++ b/rerere.c
> > @@ -219,6 +219,9 @@ 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 */
>
> Multi-line comments should have the delimiters on separate lines from
> the text. See `CodingGuidelines` under “Multi-line comments”.

done


Thank you for your feedback!

[PATCH v2] rerere: fix crash during clear

[Marcel Röthke]

When rerere_clear is called, for instance when aborting a rebase, and
the current conflict does not have a pre or postimage recorded git
crashes with a SEGFAULT in has_rerere_resolution when accessing the
status member of struct rerere_dir. This happens because scan_rerere_dir
only allocates the status field in struct rerere_dir when a post or
preimage was found. In some cases a segfault may happen even if a post
or preimage was recorded if it was not for the variant of interest and
the number of the variant that is present is lower than the variant of
interest.

Solve this by making sure the status field is large enough to
accommodate for the variant of interest so it can be accessed without
checking if it is large enough.

An alternative solution would be to always check before accessing the
status field, but I think the chosen solution aligns better with the
assumptions made elsewhere in the code.

Signed-off-by: Marcel Röthke <marcel@roethke.info>
---
Range-diff against v1:
1:  93f982d170 ! 1:  68178298fe rerere: fix crash in during clear
    @@ Metadata
     Author: Marcel Röthke <marcel@roethke.info>
     
      ## Commit message ##
    -    rerere: fix crash in during clear
    +    rerere: fix crash during clear
     
         When rerere_clear is called, for instance when aborting a rebase, and
         the current conflict does not have a pre or postimage recorded git
    @@ Commit message
         the number of the variant that is present is lower than the variant of
         interest.
     
    -    This patch solves this by making sure the status field is large enough
    -    to accommodate for the variant of interest so it can be accesses without
    +    Solve this by making sure the status field is large enough to
    +    accommodate for the variant of interest so it can be accessed without
         checking if it is large enough.
     
         An alternative solution would be to always check before accessing the
    @@ rerere.c: 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 */
    ++		/*
    ++		 * 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;
      	}

 rerere.c | 5 +++++
 1 file changed, 5 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);
-- 
2.43.2

Re: [PATCH v2] rerere: fix crash during clear

[Junio C Hamano]

Marcel Röthke <marcel@roethke.info> writes:

> When rerere_clear is called, for instance when aborting a rebase, and
> the current conflict does not have a pre or postimage recorded git
> crashes with a SEGFAULT in has_rerere_resolution when accessing the
> status member of struct rerere_dir.

I had to read this twice before realizing the reason why I found it
hard to grok was because of a missing comma between "recorded" and
"git".

> This happens because scan_rerere_dir
> only allocates the status field in struct rerere_dir when a post or
> preimage was found.

But that is not really the root cause, no?  Readers following the
above text are probably wondering why the preimage was not recorded,
when a conflict resulted in stopping a mergy-command and invoking
rerere machinery, before rerere_clear() got called.  Is that
something that usually happen?  How?  Do we have a reproduction
sequence of such a state that we can make it into a new test in
t4200 where we already have tests for "git rerere clear" and its
friends?

> In some cases a segfault may happen even if a post
> or preimage was recorded if it was not for the variant of interest and
> the number of the variant that is present is lower than the variant of
> interest.

Ditto.  What sequence of events would lead to such a state?

The answer *can* be ".git/rr-cache being a normal directory, the
user can poke around, removing files randomly, which can create such
a problematic situation", and the reproduction test *can* also be to
simulate such an end-user action, but I am asking primarily because
I want to make sure that we are *not* losing or failing to create
necessary preimage files, causing this problem to users who do not
muck with files in .git/rr-cache themselves.

Thanks.

Re: [PATCH v2] rerere: fix crash during clear

[Marcel Röthke]

On 2024-02-19 17:22:43, Junio C Hamano wrote:
> Marcel Röthke <marcel@roethke.info> writes:
>
> > When rerere_clear is called, for instance when aborting a rebase, and
> > the current conflict does not have a pre or postimage recorded git
> > crashes with a SEGFAULT in has_rerere_resolution when accessing the
> > status member of struct rerere_dir.
>
> I had to read this twice before realizing the reason why I found it
> hard to grok was because of a missing comma between "recorded" and
> "git".

fixed

> > This happens because scan_rerere_dir
> > only allocates the status field in struct rerere_dir when a post or
> > preimage was found.
>
> But that is not really the root cause, no?  Readers following the
> above text are probably wondering why the preimage was not recorded,
> when a conflict resulted in stopping a mergy-command and invoking
> rerere machinery, before rerere_clear() got called.  Is that
> something that usually happen?  How?  Do we have a reproduction
> sequence of such a state that we can make it into a new test in
> t4200 where we already have tests for "git rerere clear" and its
> friends?

I'm unfortunately not sure how it happened. I do have the initial state
of the repository and I think I know the commands that were executed,
but I could not reproduce it.

I will look into adding a test case though.

Re: [PATCH v2] rerere: fix crash during clear

[Junio C Hamano]

Marcel Röthke <marcel@roethke.info> writes:

> On 2024-02-19 17:22:43, Junio C Hamano wrote:
>> Marcel Röthke <marcel@roethke.info> writes:
>>
>> > When rerere_clear is called, for instance when aborting a rebase, and
>> > the current conflict does not have a pre or postimage recorded git
>> > crashes with a SEGFAULT in has_rerere_resolution when accessing the
>> > status member of struct rerere_dir.
>>
>> I had to read this twice before realizing the reason why I found it
>> hard to grok was because of a missing comma between "recorded" and
>> "git".
>
> fixed
> ...
> I'm unfortunately not sure how it happened. I do have the initial state
> of the repository and I think I know the commands that were executed,
> but I could not reproduce it.
>
> I will look into adding a test case though.

It has been about a month.  Any new development on this topic?

Thanks.  

Re: [PATCH v2] rerere: fix crash during clear

[Marcel Röthke]

On 2024-03-24 14:51:17, Junio C Hamano wrote:
> Marcel Röthke <marcel@roethke.info> writes:
>
> > On 2024-02-19 17:22:43, Junio C Hamano wrote:
> >> Marcel Röthke <marcel@roethke.info> writes:
> >>
> >> > When rerere_clear is called, for instance when aborting a rebase, and
> >> > the current conflict does not have a pre or postimage recorded git
> >> > crashes with a SEGFAULT in has_rerere_resolution when accessing the
> >> > status member of struct rerere_dir.
> >>
> >> I had to read this twice before realizing the reason why I found it
> >> hard to grok was because of a missing comma between "recorded" and
> >> "git".
> >
> > fixed
> > ...
> > I'm unfortunately not sure how it happened. I do have the initial state
> > of the repository and I think I know the commands that were executed,
> > but I could not reproduce it.
> >
> > I will look into adding a test case though.
>
> It has been about a month.  Any new development on this topic?
>
> Thanks.

No, I have been otherwise occupied. But I plan to get back to this soon. Probably this week, next week at the latest.

Re: [PATCH v2] rerere: fix crash during clear

[Marcel Röthke]

After reproducing this without manually corrupting the database, I
noticed that the summary line is no longer accurate. Because it fixes
two SEGFAULTs and one of them does not happen during clear. Can I change this
in v3 or should I start a new thread?

Re: [PATCH v2] rerere: fix crash during clear

[Junio C Hamano]

Marcel Röthke <marcel@roethke.info> writes:

> After reproducing this without manually corrupting the database, I
> noticed that the summary line is no longer accurate. Because it fixes
> two SEGFAULTs and one of them does not happen during clear. Can I change this
> in v3 or should I start a new thread?

Updating the title in the course of evolution of a topic/patch is
perfectly normal.  Please do so in v3 as continuation of the
previous round.

Thanks.

[PATCH v3] rerere: fix crashes due to unmatched opening conflict markers

[Marcel Röthke]

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

Re: [PATCH v3] rerere: fix crashes due to unmatched opening conflict markers

[Junio C Hamano]

Marcel Röthke <marcel@roethke.info> writes:

> 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.

Nicely diagnosed.  Yes, such a corrupt preimage should not enter the
rerere database as it is unusable for future replaying of the
resolution.  rerere should be prepared to deal with such an input
more gracefully.

> 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.

Thanks.

> 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.

I somehow doubt that "parse stray markers" is something we _want_ to
do in the first place.  Is it "the root cause" that we refuse/reject
such a corrupt input from entering the rerere database?  To me, it
seems like that the issue is that we do not do a very good job
rejecting them, keeping the internal state consistent.

>  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);

Both the fix, and the newly added tests, look great to me.

Thanks.  Will queue.

Re: [PATCH v3] rerere: fix crashes due to unmatched opening conflict markers

[Junio C Hamano]

Marcel Röthke <marcel@roethke.info> writes:

> +test_expect_success 'rerere does not crash with unmatched conflict marker' '
> +	git config rerere.enabled true &&
> ...
> +	git rebase --continue
> +'
> +

This one fails, either standalone or when merged to 'seen'.

A sample CI run that failed can be seen here (you probably need to
be logged in to view it):

https://github.com/git/git/actions/runs/8694652245/job/23844028985#step:5:1894

Re: [PATCH v3] rerere: fix crashes due to unmatched opening conflict markers

[Marcel Röthke]

On 2024-04-15 13:15:35, Junio C Hamano wrote:
> Marcel Röthke <marcel@roethke.info> writes:
>
> > +test_expect_success 'rerere does not crash with unmatched conflict marker' '
> > +	git config rerere.enabled true &&
> > ...
> > +	git rebase --continue
> > +'
> > +
>
> This one fails, either standalone or when merged to 'seen'.
>
> A sample CI run that failed can be seen here (you probably need to
> be logged in to view it):
>
> https://github.com/git/git/actions/runs/8694652245/job/23844028985#step:5:1894

Yes, sorry that I missed that. The last git rebase command in that test needs
a test_must_fail because we expect a merge conflict.
Will be fixed in v4.

[PATCH v4] rerere: fix crashes due to unmatched opening conflict markers

[Marcel Röthke]

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 v3:
  diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
  index 1e80f76860..b0a3e84984 100755
  --- a/t/t4200-rerere.sh
  +++ b/t/t4200-rerere.sh
  @@ -731,7 +731,7 @@ test_expect_success 'rerere does not crash with unmatched conflict marker' '
   	bar
   	EOF
   	git add test &&
  -	git rebase --continue
  +	test_must_fail 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..b0a3e84984 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 &&
+	test_must_fail git rebase --continue
+'
+
 test_done
--
2.44.0