From c00ae9d699eb2023d304543bd9d776228f31e354 Mon Sep 17 00:00:00 2001 From: Jiri Kuncar Date: Fri, 11 Sep 2026 16:16:04 +0200 Subject: [PATCH] pull: avoid segfault when commit lookup fails get_can_ff() and already_up_to_date() pass the result of lookup_commit_reference() straight to commit_list_insert() and repo_is_descendant_of() without checking it. When the object behind HEAD or one of the merge heads cannot be parsed, e.g. because a loose object was left truncated by a fetch or gc racing on the same repository, lookup_commit_reference() returns NULL and "git pull" segfaults instead of reporting the corruption. Treat a failed lookup as "cannot fast-forward" and "not up to date", so that the caller falls through to the normal merge path, which already diagnoses the broken object and fails cleanly. An alternative would be to report the breakage at each lookup site, which could give a more precise diagnosis. The minimal guards are preferred because they do no more than is needed to avoid the crash, and will be easy to drop once "git pull" is reworked to resolve object names into commit objects early and pass those around, at which point there will not be multiple lookups of the same object name to guard in the first place. The test corrupts the loose object in place rather than removing it: a missing object that is still recorded in the commit-graph is caught by the consistency check in fetch-pack before "git pull" reaches the fast-forward check, so removing it would not exercise the crash. Signed-off-by: Jiri Kuncar --- builtin/pull.c | 10 +++++++++- t/t5520-pull.sh | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/builtin/pull.c b/builtin/pull.c index db3ee0aab3ed91..80e79daeb9a3a5 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -800,8 +800,12 @@ static int get_can_ff(struct object_id *orig_head, orig_merge_head = &merge_heads->oid[0]; head = lookup_commit_reference(the_repository, orig_head); - commit_list_insert(head, &list); + if (!head) + return 0; merge_head = lookup_commit_reference(the_repository, orig_merge_head); + if (!merge_head) + return 0; + commit_list_insert(head, &list); ret = repo_is_descendant_of(the_repository, merge_head, list); commit_list_free(list); if (ret < 0) @@ -820,12 +824,16 @@ static int already_up_to_date(struct object_id *orig_head, struct commit *ours; ours = lookup_commit_reference(the_repository, orig_head); + if (!ours) + return 0; for (size_t i = 0; i < merge_heads->nr; i++) { struct commit_list *list = NULL; struct commit *theirs; int ok; theirs = lookup_commit_reference(the_repository, &merge_heads->oid[i]); + if (!theirs) + return 0; commit_list_insert(theirs, &list); ok = repo_is_descendant_of(the_repository, ours, list); commit_list_free(list); diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 27f38ab3c83c85..b3ab8f4c9404ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -888,4 +888,31 @@ test_expect_success 'git pull --rebase against local branch' ' test_cmp expect file2 ' +test_expect_success 'pull does not crash when a merge head does not resolve' ' + test_when_finished "rm -rf up dn" && + git init up && + ( + cd up && + test_commit base && + git switch -c sideA && + test_commit a && + git switch -c sideB base && + test_commit b + ) && + git clone up dn && + ( + cd dn && + git commit-graph write --reachable && + oid=$(git rev-parse refs/remotes/origin/sideA) && + obj=.git/objects/$(test_oid_to_path "$oid") && + + # Corrupt the object instead of removing it: a missing + # object that is still in the commit-graph is caught by + # fetch before pull ever reaches the fast-forward check. + rm -f "$obj" && + echo garbage >"$obj" && + test_must_fail git pull --no-rebase origin sideA sideB + ) +' + test_done