From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got fetch -d crash
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 10 Sep 2021 23:13:33 +0200

On Thu, Sep 09, 2021 at 03:12:49PM +0200, Omar Polo wrote:
> Hello,
> 
> I've spotted a segfault during `got fetch -d'
> 
> In got.c:2040
> 
>    2039                 TAILQ_FOREACH(pe, their_refs, entry) {
>    2040                         if (strcmp(local_refname, pe->path) == 0)
>    2041                                 break;
>    2042                 }
> 
> local_refname is NULL and strcmp crashes.  I couldn't get a
> corefile thought
> 
> --8<---------------cut here---------------start------------->8---
> % egdb got
> (gdb) r fetch -d
> Starting program: /home/op/bin/got fetch -d
> [New process 79176]
> Connecting to "origin" git@omarpolo.com
> Already up-to-date
> 
> Thread 1 received signal SIGSEGV, Segmentation fault.
> strcmp () at /usr/src/lib/libc/arch/amd64/string/strcmp.S:45
> 45              movq    8(%rdi),%rax
> (gdb) bt
> #0  strcmp () at /usr/src/lib/libc/arch/amd64/string/strcmp.S:45
> #1  0x000006cbf275413a in delete_missing_refs (their_refs=0x7f7ffffecf00,
>     their_symrefs=0x7f7ffffecef0, remote=0x6cdfe936250, verbosity=0,
>     repo=0x6cdfe917070) at got.c:2040
> #2  0x000006cbf274475c in cmd_fetch (argc=0, argv=0x7f7ffffed0a0) at got.c:2538
> #3  0x000006cbf274105f in main (argc=2, argv=0x7f7ffffed090) at got.c:252
> (gdb) f 1
> #1  0x000006cbf275413a in delete_missing_refs (their_refs=0x7f7ffffecf00,
>     their_symrefs=0x7f7ffffecef0, remote=0x6cdfe936250, verbosity=0,
>     repo=0x6cdfe917070) at got.c:2040
> 2040                            if (strcmp(local_refname, pe->path) == 0)
> (gdb) p local_refname
> $1 = 0x0
> --8<---------------cut here---------------end--------------->8---
> 
> A plain `got fetch' works fine.
> 
> I'm using got compiled from the latest commit 9b21d88
> 
> Background: (probably not interesting but who knows)
> 
> I've deleted a tag server side with `git tag -d 1.0' followed by a `git
> gc', then I wanted to pull the change.  After a `got fetch' the tag 1.0
> is still there, so I tried with -d and found the crash.
> 
> Here's the repo config in case it's useful:
> 
> 	% cat /home/op/git/vc-got.git/config
> 	[core]
> 		repositoryformatversion = 0
> 		filemode = true
> 		bare = true
> 	[remote "origin"]
> 		url = ssh://git@omarpolo.com/vc-got.git
> 		fetch = +refs/*:refs/*
> 		mirror = true
> 	[remote "github"]
> 		url = ssh://git@github.com/omar-polo/vc-got
> 		fetch = +refs/heads/*:refs/remotes/github/*
> 
> Thanks,
> 
> Omar Polo
> 
> 

Thank you, Omar.

The patch below fixes the bug and adds a test which reproduces the crash.
Does this work for you?

diff 678d8c1fe20f1eab5a3568135b24f00c9d22c33c /home/stsp/src/got
blob - 5f3984d14029304bc35fa0a2ae1caeff8de74a69
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -2020,8 +2020,11 @@ delete_missing_refs(struct got_pathlist_head *their_re
 
 	TAILQ_FOREACH(re, &my_refs, entry) {
 		const char *refname = got_ref_get_name(re->ref);
+		const char *their_refname;
 
-		if (!remote->mirror_references) {
+		if (remote->mirror_references) {
+			their_refname = refname;
+		} else {
 			if (strncmp(refname, remote_namespace,
 			    strlen(remote_namespace)) == 0) {
 				if (strcmp(refname + strlen(remote_namespace),
@@ -2034,17 +2037,19 @@ delete_missing_refs(struct got_pathlist_head *their_re
 				}
 			} else if (strncmp(refname, "refs/tags/", 10) != 0)
 				continue;
-		}
 
+			their_refname = local_refname;
+		}
+	
 		TAILQ_FOREACH(pe, their_refs, entry) {
-			if (strcmp(local_refname, pe->path) == 0)
+			if (strcmp(their_refname, pe->path) == 0)
 				break;
 		}
 		if (pe != NULL)
 			continue;
 
 		TAILQ_FOREACH(pe, their_symrefs, entry) {
-			if (strcmp(local_refname, pe->path) == 0)
+			if (strcmp(their_refname, pe->path) == 0)
 				break;
 		}
 		if (pe != NULL)
blob - e0587383f411d35c4be341e74b562fba63fbfbf8
file + regress/cmdline/fetch.sh
--- regress/cmdline/fetch.sh
+++ regress/cmdline/fetch.sh
@@ -509,6 +509,102 @@ test_fetch_delete_branch() {
 
 }
 
+test_fetch_delete_branch_mirror() {
+	local testroot=`test_init fetch_delete_branch_mirror`
+	local testurl=ssh://127.0.0.1/$testroot
+	local commit_id=`git_show_head $testroot/repo`
+
+	got branch -r $testroot/repo -c $commit_id foo
+	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
+	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
+	local tag_id=`got ref -r $testroot/repo -l \
+		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+
+	got clone -a -m -q $testurl/repo $testroot/repo-clone
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got clone command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/foo: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	# refs/hoo/boo/zoo is missing because it is outside of refs/heads
+	echo "refs/tags/1.0: $tag_id" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got branch -r $testroot/repo -d foo >/dev/null
+
+	got fetch -q -r $testroot/repo-clone
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got fetch command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/foo: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	# refs/hoo/boo/zoo is missing because it is outside of refs/heads
+	echo "refs/tags/1.0: $tag_id" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got fetch -d -q -r $testroot/repo-clone > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got fetch command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	# refs/heads/foo is now deleted
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	# refs/hoo/boo/zoo is missing because it is outside of refs/heads
+	echo "refs/tags/1.0: $tag_id" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+
+}
+
 test_fetch_update_tag() {
 	local testroot=`test_init fetch_update_tag`
 	local testurl=ssh://127.0.0.1/$testroot
@@ -1191,6 +1287,7 @@ run_test test_fetch_branch
 run_test test_fetch_all
 run_test test_fetch_empty_packfile
 run_test test_fetch_delete_branch
+run_test test_fetch_delete_branch_mirror
 run_test test_fetch_update_tag
 run_test test_fetch_reference
 run_test test_fetch_replace_symref