threads / bug / 61513

bug: Segfault with git diff

Subject: bug: Segfault with git diff

## tl;dr

7 messages between May 19, 2024 and May 1, 2025.

replies: 6people: 4as markdown or json

jake roggenbuck· May 19, 2024, 03:02 UTC · lore
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
I have found a segfault when running git diff.
> What did you do before the bug happened? (Steps to reproduce your issue)
1. After running `git diff`, git showed me that object files were empty.
2. I deleted the empty object files, and ran `git diff` again.
3. I continued deleting the empty files until `git diff` segfaulted.
> What did you expect to happen? (Expected behavior)
An error message of some type or a graceful exit.
> What happened instead? (Actual behavior)
A segmentation fault.
`Segmentation fault (core dumped)`
> What's different between what you expected and what actually happened?
Instead of closing gracefully, there was a segmentation fault.
> Anything else you want to add:

git log displays: fatal: bad object HEAD

git branch displays: fatal: missing object 7610511b1b4db888e8e6bb8d0ff158f932961345 for refs/heads/main

Neither log nor branch causes the segfault.

[System Info] git version: git version 2.45.1 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 6.7.3-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 01 Feb 2024 10:30:35 +0000 x86_64 compiler info: gnuc: 14.1 libc info: glibc: 2.39 $SHELL (typically, interactive shell): /bin/bash

[Enabled Hooks]

Best, Jake Roggenbuck

Jake Roggenbuck· Jan 8, 2025, 06:01 UTC · re: jake roggenbuck · lore

[PATCH 0/1] Exit on invalid diff status of diff_filepair

Hi all,

Git showed that object files were empty, so I manually removed them. After removing a few object files, I ran `git diff` and I got a segfault.

I have since narrowed down the source of the error to a usage of `diff_filepair`. I also noticed that the `status` field of `diff_filepair` does not get set and stays a zero value. My fix is to check for that invalid status character and exit gracefully with an error message.

I modeled the behavior of this patch after what `git log` does in the same situation. `git log` will exit with a message that reads `bad object HEAD`.

I initially found this segfault when using git while working on a school assignment. I am really interested in helping fix this segfault. I'd be happy to hear any feedback.

Thank you!
Jake Roggenbuck (1):
  Exit on invalid diff status of diff_filepair
 diff.c | 4 ++++
 1 file changed, 4 insertions(+)
-- 
2.47.0
Jake Roggenbuck· Jan 8, 2025, 06:01 UTC · re: Jake Roggenbuck · lore

[PATCH 1/1] Exit on invalid diff status of diff_filepair

Add a check for the invalid status of `0` for `diff_filepair` when certain object files are missing. When these object files are missing, 'git log' returns 'fatal: bad object HEAD' but 'git diff' segfaults.

Normally, the `diff_filepair` status should be a character, but when object files are removed, status becomes a zero character which isn't listed as one of the possible status letters in `Documentation/diff-format.txt`.

This patch checks for that invalid status character and gracefully exits with an error message.

Signed-off-by: Jake Roggenbuck <jakeroggenbuck2@gmail.com>
---
 diff.c | 4 ++++
 1 file changed, 4 insertions(+)
diff --git a/diff.c b/diff.c
index 6c96154fed..aeab1ac445 100644
--- a/diff.c
+++ b/diff.c
@@ -7018,6 +7018,10 @@ void diff_queued_diff_prefetch(void *repository)
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
+
+		if (!p->status)
+			die("invalid diff status");
+
 		diff_add_if_missing(repo, &to_fetch, p->one);
 		diff_add_if_missing(repo, &to_fetch, p->two);
 	}

-- 
2.47.0
Jake Roggenbuck· Apr 30, 2025, 18:50 UTC · re: Jake Roggenbuck · lore

Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair

Hi all,

Has anyone gotten a chance to take a look at this simple fix? When git is built of the main branch, it still segfaults when there is an invalid diff_filepair.

    git (main) $ make
    <make output omitted>
    git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/
    hw3-skeleton-broken (main) $ ~/Build/git/git diff
    Segmentation fault (core dumped)
    hw3-skeleton-broken (main) $
Let me know if you have any feedback or suggestions.
Signed-off-by: Jake Roggenbuck <jakeroggenbuck2@gmail.com>
Eric Sunshine· May 1, 2025, 06:16 UTC · re: Jake Roggenbuck · lore

Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair

On Wed, Apr 30, 2025 at 2:53 PM Jake Roggenbuck <jakeroggenbuck2@gmail.com> wrote:

Show 11 quoted lines
> Has anyone gotten a chance to take a look at this simple fix?
> When git is built of the main branch, it still segfaults when there is an invalid diff_filepair.
>
>     git (main) $ make
>     <make output omitted>
>     git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/
>     hw3-skeleton-broken (main) $ ~/Build/git/git diff
>     Segmentation fault (core dumped)
>     hw3-skeleton-broken (main) $
>
> Let me know if you have any feedback or suggestions.

Do you have a reproduction recipe which demonstrates the problem which your patch fixes? Including the recipe in the patch's commit message would help reviewers better understand the circumstances under which the crash occurs since the descriptions provided by both the original problem report[1] and the submitted patch[2] seem rather nebulous[3].

More importantly, if you have a reproduction recipe, then it can be used as the basis for creating a test which should accompany the patch (and which should be added to one of the `t/t40xx-*.sh` files). We can help you convert the reproduction recipe into a test if desired.

FOOTNOTES
[1]: https://lore.kernel.org/git/CAEUC8gmgq_yViedLGHOeSyvR9rQK+O-8Fh9wzds=2+326ngUjw@mail.gmail.com/
[2]: https://lore.kernel.org/git/20250108060151.7218-2-jakeroggenbuck2@gmail.com/

[3]: It is not clear, for instance, what an "object file" is in the provided description. Are you referring to one of the files under `.git/objects/`?

Junio C Hamano· May 1, 2025, 13:23 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 17 quoted lines
>>     git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/
>>     hw3-skeleton-broken (main) $ ~/Build/git/git diff
>>     Segmentation fault (core dumped)
>>     hw3-skeleton-broken (main) $
>>
>> Let me know if you have any feedback or suggestions.
>
> Do you have a reproduction recipe which demonstrates the problem which
> your patch fixes? Including the recipe in the patch's commit message
> would help reviewers better understand the circumstances under which
> the crash occurs since the descriptions provided by both the original
> problem report[1] and the submitted patch[2] seem rather nebulous[3].
>
> More importantly, if you have a reproduction recipe, then it can be
> used as the basis for creating a test which should accompany the patch
> (and which should be added to one of the `t/t40xx-*.sh` files). We can
> help you convert the reproduction recipe into a test if desired.
Nicely put.

It is a bug _elsewhere_ in the code for the .status member to be unassigned when the control reaches that point, so a patch to exit after that happens is not all that interesting. Instead of exiting there, we would want to see a patch against the place where it should have set the .status member but fails to do so.

Maybe with a corrupted repository, some of the blob objects we read for comparison may fail to load and the function may be taking an early return instead of complaining and dying upon unreadable blob (I am not saying that is the only or even a likely case; just giving an example situation for illustrate the point), in which case we would want to fix _that_ code to complain and die properly.

Thanks.

← back to recent threads