From: Omar Polo Subject: Re: got fetch -d crash To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Sat, 11 Sep 2021 09:53:07 +0200 Stefan Sperling writes: > 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? With this I can't trigger the crash anymore, thanks! > 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