From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd crash in got_gotweb_blame_cb
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Thu, 18 Aug 2022 08:46:10 -0600

On Thu, Aug 18, 2022 at 12:25:37PM +0200, Omar Polo wrote:
> On 2022/08/17 15:49:57 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > On Tue, Aug 16, 2022 at 11:42:46PM +0200, Omar Polo wrote:
> > > found that some hours ago gotwebd crashed.  I'm running gotwebd as of
> > > bf80b15220f51490025e916633cdd70816113604 ("gotwebd: fix alternate
> > > colors in tree entries") plus the fcgi_printf and the fix for the
> > > double free on gotweb_free_querystring.
> > > 
> > > can't study the stacktrace now, will do that tomorrow when I have
> > > time, but in the meantime I'm sharing it in case it rings some bells :)
> > 
> > Try the patch below.
> > If this is indeed the bug, then it also exists in got/got.c.
> 
> I found how to reproduce the bug with `got blame', but interestingly
> it doesn't happen with `tog blame'!
> 
> The diff doesn't help
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x000007ec6721f72f in blame_cb (arg=0x7f7fffff2d58, nlines=512, lineno=512,
>     commit=0x7eeb31bf780, id=0x7eed27b1a40) at /home/op/w/got/got/got.c:5369
> 5369            while (bline->annotated && a->lineno_cur <= a->nlines) {
> (gdb) p *bline
> Cannot access memory at address 0x7ef290b3000
> (gdb) p *a
> $1 = {lines = 0x7ef290ae000, nlines = 512, nlines_prec = 3, lineno_cur = 513,
>   line_offsets = 0x7ee6775a000, f = 0x7ee68a93028 <usual+152>, repo = 0x7eeb31cb520}
> 
> > diff /home/stsp/src/got
> > commit - 4fcc9f7404ca2e0dd2ee085f09d6246587c6c503
> > path + /home/stsp/src/got
> > blob - 1c274b225d0e710d16528c4b9970348bb3bec74e
> > file + gotwebd/got_operations.c
> > --- gotwebd/got_operations.c
> > +++ gotwebd/got_operations.c
> > @@ -1334,7 +1334,7 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
> >  		goto done;
> >  	}
> >  
> > -	while (bline->annotated) {
> > +	while (bline->annotated && a->lineno_cur <= a->nlines) {
> 
> I can avoid the crash by changing the <= to < but then the last line
> is not print...  There has to be an off by one somewhere, I don't have
> the time right now to track this further, sorry :(
> 
> I'm able to reproduce it using
> 
> $ got blame -c 72e7fbbcb7740a86e44d043727782b50356dd69d ChangeLog
> 
> in my gmid repo, available at:
> 
> git://git.omarpolo.com/gmid.git
> https://git.omarpolo.com/gmid.git
> 

Try this. I think stsp just had it reversed.

-- 

Tracey Emery

diff /home/tracey/src/got
commit - bf80b15220f51490025e916633cdd70816113604
path + /home/tracey/src/got
blob - 39827e91edca9da87bdc0ce98c9a77260b22dd4e
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -5368,7 +5368,7 @@ blame_cb(void *arg, int nlines, int lineno,
 		goto done;
 	}
 
-	while (bline->annotated) {
+	while (a->lineno_cur <= a->nlines && bline->annotated) {
 		char *smallerthan, *at, *nl, *committer;
 		size_t len;