git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] pull, fetch: fix segfault in --set-upstream option

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 19, 2021, 14:30 UTC
Message-ID
<patch-1.1-2d8f3e59e1f-20210719T142808Z-avarab@gmail.com>
In-Reply-To
<8636b96be256b47d207e543995abbecde9ca5055.camel@fnusa.cz>

Fix a segfault in the --set-upstream option added in 24bc1a12926 (pull, fetch: add --set-upstream option, 2019-08-19) added in v2.24.0.

The code added there did not do the same checking we do for "git branch" itself since 8efb8899cfe (branch: segfault fixes and validation, 2013-02-23), which in turn fixed the same sort of segfault I'm fixing now in "git branch --set-upstream-to", see 6183d826ba6 (branch: introduce --set-upstream-to, 2012-08-20).

The error message I'm adding here is an amalgamation of the error added for "git branch" in 8efb8899cfe, and the error output install_branch_config() itself emits, i.e. it trims "refs/heads/" from the name and says "branch X on remote", not "branch refs/heads/X on remote".

Reported-by: Clemens Fruhwirth <clemens@endorphin.org>
Reported-by: Jan Pokorný <poki@fnusa.cz>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
On Mon, Jul 19 2021, Jan Pokorný wrote:
Show 10 quoted lines
> Hello,
>
> Clemens Fruhwirth píše v Po 05. 07. 2021 v 17:46 +0200:
>> What did you do before the bug happened? (Steps to reproduce your
>> issue)
>> 
>> Run "git pull origin nixos-unstable --rebase --set-upstream"
>> on a repo that had no branch set, e.g. when running "git branch" gave
>> "* (no branch)"
>> 

Thanks for the report both & sorry that this fell through the cracks for so long.

The bug itself seems rather obvious given the benefit of that reproduction scenario & backtrace, as noted above we've been playing whack-a-mole with a related segfault since 2012.

It would be nice if you could test this patch and confirm, but given the trivality of the segfault it shouldn't be necessary, I'm confident that this fixes the bug you two reported. Thanks both!

 builtin/fetch.c         | 11 +++++++++++
 t/t5553-set-upstream.sh | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9191620e50c..c2eac8f15e5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1623,6 +1623,17 @@ static int do_fetch(struct transport *transport,
 			}
 		}
 		if (source_ref) {
+			if (!branch) {
+				const char *shortname = NULL;
+				if (!skip_prefix(source_ref->name,
+						 "refs/heads/", &shortname))
+					shortname = source_ref->name;
+				    
+				die(_("could not set upstream of HEAD to '%s' from '%s' when "
+				      "it does not point to any branch."),
+				    shortname, transport->remote->name);
+			}
+
 			if (!strcmp(source_ref->name, "HEAD") ||
 			    starts_with(source_ref->name, "refs/heads/"))
 				install_branch_config(0,
diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
index b1d614ce18c..ae90554645e 100755
--- a/t/t5553-set-upstream.sh
+++ b/t/t5553-set-upstream.sh
@@ -91,6 +91,17 @@ test_expect_success 'fetch --set-upstream with valid URL sets upstream to URL' '
 	check_config_missing other2
 '
 
+test_expect_success 'fetch --set-upstream with a detached HEAD' '
+	git checkout HEAD^0 &&
+	test_when_finished "git checkout -" &&
+	cat >expect <<-\EOF &&
+	fatal: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+	EOF
+	test_must_fail git fetch --set-upstream upstream main 2>actual.raw &&
+	grep ^fatal: actual.raw >actual &&
+	test_cmp expect actual
+'
+
 # tests for pull --set-upstream
 
 test_expect_success 'setup bare parent pull' '
@@ -178,4 +189,15 @@ test_expect_success 'pull --set-upstream with valid URL and branch sets branch'
 	check_config_missing other2
 '
 
+test_expect_success 'pull --set-upstream with a detached HEAD' '
+	git checkout HEAD^0 &&
+	test_when_finished "git checkout -" &&
+	cat >expect <<-\EOF &&
+	fatal: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+	EOF
+	test_must_fail git pull --set-upstream upstream main 2>actual.raw &&
+	grep ^fatal: actual.raw >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.32.0.874.ge7a9d58bfcf
Previous: Jan PokornýNext: Junio C Hamano
Message 3 of 13 in “git pull --set-upstream segfaults on branchless repo.”
  1. Clemens FruhwirthJul 5, 2021
  2. Jan PokornýJul 19, 2021
  3. pull, fetch: fix segfault in --set-upstream optionÆvar Arnfjörð Bjarmason, Jul 19, 2021
  4. Junio C HamanoJul 19, 2021
  5. pull, fetch: fix segfault in --set-upstream optionÆvar Arnfjörð Bjarmason, Aug 23, 2021
  6. Clemens FruhwirthAug 24, 2021
  7. Ævar Arnfjörð BjarmasonAug 24, 2021
  8. pull, fetch: fix segfault in --set-upstream optionÆvar Arnfjörð Bjarmason, Aug 30, 2021
  9. Junio C HamanoAug 30, 2021
  10. pull, fetch: fix segfault in --set-upstream optionÆvar Arnfjörð Bjarmason, Aug 31, 2021
  11. Junio C HamanoAug 31, 2021
  12. Ævar Arnfjörð BjarmasonAug 31, 2021
  13. Junio C HamanoSep 1, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.