From: Mark Jamsek <mark@jamsek.com>
Subject: Re: gotd flush packets
To: gameoftrees@openbsd.org
Date: Sat, 21 Jan 2023 19:48:56 +1100

On 23-01-21 09:15AM, Stefan Sperling wrote:
> On Fri, Jan 20, 2023 at 07:57:59PM +0100, Stefan Sperling wrote:
> > Someone did a silly thing against our public git mirror:
> > 
> > Jan 20 19:20:23 florabel gotd[74429]: received flush-pkt from uid 1002
> > Jan 20 19:20:43 florabel last message repeated 251236 times
> > Jan 20 19:20:43 florabel gotd[70339]: list-refs request from uid 1002
> > Jan 20 19:20:43 florabel gotd[74429]: received flush-pkt from uid 1002
> > Jan 20 19:21:14 florabel last message repeated 386788 times
> > Jan 20 19:21:43 florabel last message repeated 367291 times
> 
> Looking closer in less of a hurry, I found several instances of this
> in the server logs. This is not being triggered on purpose, but happens
> because gotd can keep reading from a pipe after EOF in serve_read().
> The patch below should fix this by returning an EOF error code when
> read(2) indicates EOF.
> 
> Also, make gotsh only forward flush packets to the session process
> if the client is currrently expected to send want or have lines.
> 
> Passes regress tests, and my manual tests are still working.
> 
> OK?

Nice catch, ok

> (Regardless, I still want to apply my other patch which detects
> mulitple flush packets, since it a local user might be connecting
> to the socket directly without gotsh.)
> 
> diff /home/stsp/src/got
> commit - 7713cc5e4f5544e81909670d592e89526ed86c9b
> path + /home/stsp/src/got
> blob - 4330e9e71a7dcd41735ee2f4222e4f39e824d121
> file + lib/pkt.c
> --- lib/pkt.c
> +++ lib/pkt.c
> @@ -36,7 +36,7 @@ got_pkt_readn(ssize_t *off, int fd, void *buf, size_t 
>  		if (r == -1)
>  			return got_error_from_errno("read");
>  		if (r == 0)
> -			return NULL;
> +			return got_error(GOT_ERR_EOF);
>  		*off += r;
>  	}
>  	return NULL;
> blob - ff9cd1e8a402bfed028e0aa1db7d6cf94c159eb4
> file + lib/serve.c
> --- lib/serve.c
> +++ lib/serve.c
> @@ -928,9 +928,13 @@ serve_read(int infd, int outfd, int gotd_sock, const c
>  				goto done;
>  			}
>  
> -			err = forward_flushpkt(&ibuf);
> -			if (err)
> -				goto done;
> +			if (curstate == STATE_EXPECT_WANT ||
> +			    curstate == STATE_EXPECT_MORE_WANT ||
> +			    curstate == STATE_EXPECT_HAVE) {
> +				err = forward_flushpkt(&ibuf);
> +				if (err)
> +					goto done;
> +			}
>  			if (curstate == STATE_EXPECT_HAVE && !have_ack) {
>  				err = send_nak(outfd, chattygot);
>  				if (err)
> 
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68