From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotd segfault
To: Mark Jamsek <mark@jamsek.com>
Cc: Mikhail <mp39590@gmail.com>, gameoftrees@openbsd.org
Date: Mon, 1 May 2023 14:44:47 +0200

On Mon, May 01, 2023 at 10:37:24PM +1000, Mark Jamsek wrote:
> On 23-05-01 02:51PM, Mikhail wrote:
> > On Mon, May 01, 2023 at 02:47:35PM +0300, Mikhail wrote:
> > > core:~# egdb /home/misha/bin/gotd
> > > [...]
> > > Reading symbols from /home/misha/bin/gotd...
> > > (gdb) run -vvvvd
> > > Starting program: /home/misha/bin/gotd -vvvvd
> > > gotd: socket: /var/run/gotd.sock
> > > gotd: user: misha
> > > listen: gotd_accept: new client connected on fd 6 uid 1000 gid 1000
> > > parent: recv_connect: new client uid 1000 connected on fd 5
> > > parent: list-refs request from uid 1000
> > > parent: starting auth for uid 1000 repository src
> > > parent: proc auth /var/git/src.git is on fd 7
> > > auth /var/git/src.git: authentication request received
> > > auth /var/git/src.git: authenticating uid 1000 gid 1000
> > > auth /var/git/src.git: src: Permission denied
> > > parent: uid 1000: src: Permission denied
> > > parent: uid 1000: disconnecting
> > > parent: waiting for child PID 25599 to terminate
> > > auth /var/git/src.git: shutting down
> > > 
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0x00000b9ad943f762 in gotd_dispatch_auth_child (fd=7, event=2, arg=0xb9d122b0510) at /home/misha/work/got/gotd/gotd.c:1143
> > > 1143                    log_warnx("uid %d: %s", client->euid, err->msg);
> > > (gdb) bt
> > > #0  0x00000b9ad943f762 in gotd_dispatch_auth_child (fd=7, event=2, arg=0xb9d122b0510) at /home/misha/work/got/gotd/gotd.c:1143
> > > #1  0x00000b9cf77db41f in event_process_active (base=0xb9d512f9760) at /usr/src/lib/libevent/event.c:333
> > > #2  event_base_loop (base=0xb9d512f9760, flags=<optimized out>) at /usr/src/lib/libevent/event.c:483
> > > #3  0x00000b9ad943cb43 in main (argc=0, argv=0x78b213c7bff8) at /home/misha/work/got/gotd/gotd.c:1932
> > > (gdb) p client
> > > $1 = (struct gotd_client *) 0xb9cdc05a000
> > > (gdb) p err
> > > $2 = (const struct got_error *) 0xb9ad9514030 <custom_errors>
> > > (gdb) p err->msg
> > > $3 = 0xb9ad9514040 <custom_errors+16> "src: Permission denied"
> > > (gdb) p client->euid
> > > Cannot access memory at address 0xb9cdc06a170
> > > 
> > > Is this info enough?
> > 
> > The problem was that in gotd.conf I had:
> > 
> > repository "src" {
> >         path "/var/git/src.git"
> > #        permit rw flan_hacker
> > #        permit rw :developers
> >         permit ro anonymous
> > 
> >         protect branch "main"
> >         protect tag namespace "refs/tags/"
> > }
> > 
> > Replacing anonymous with misha made gotd work, but with anonymous it
> > crashes.
> 
> I think we might just want to return here as client is freed in
> disconnect(), which is either called in the else case or called from
> disconnect_on_error(), and if we goto done here instead of returning we
> will perform an invalid read on client whether there is an error or not.
> Plus, the error logged in done is already logged in
> disconnect_on_error().

Yes, this makes sense. ok

> diff /home/mark/src/got
> commit - 79cd0a74be4fd59c3cbfc4f35772ba4336b950be
> path + /home/mark/src/got
> blob - 886f92ecc38e22a85b9de751ac0be7ba7d14d98b
> file + gotd/gotd.c
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -1121,7 +1121,7 @@ gotd_dispatch_auth_child(int fd, short event, void *ar
>  			disconnect_on_error(client, err);
>  		else
>  			disconnect(client);
> -		goto done;
> +		return;
>  	}
>  
>  	repo = gotd_find_repo_by_name(client->auth->repo_name, &gotd);
> 
> -- 
> Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
> GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68
>