The tale of a TCP bug

[John Baldwin]

On Saturday, April 02, 2011 7:58:23 am Stefan `Sec` Zehl wrote:
> Hi I'm back :)
> 
> On Fri, Apr 01, 2011 at 01:40 +0200, Stefan `Sec` Zehl wrote:
> > I'll of course monitor this value and report back if I ever see it
> > increase :-)
> 
> It did:
> 
> | ice:~>uptime
> |  1:45PM  up 2 days, 17:01, 0 users, load averages: 1.29, 0.98, 0.60
> | ice:~>sysctl net.inet.tcp.adv_neg
> | net.inet.tcp.adv_neg: 120
> | ice:~>
> 
> I currently have no idea why. But I think it would be a good idea to fix
> that adv calculation on 64bit for the negative case anyway.
> 
> As my original attempt with a (long) cast was frowned upon, maybe
> something like what OpenBSD did in r1.15 / 1998?
> 
> http://www.openbsd.org/cgi-bin/cvsweb/src/sys/netinet/tcp_output.c.diff?r1=1.14;r2=1.15
> 
> --- tcp_output.c.pre    2011-04-02 13:50:32.000000000 +0200
> +++ tcp_output.c        2011-04-02 13:50:35.000000000 +0200
> @@ -575,7 +575,7 @@
>                  * taking into account that we are limited by
>                  * TCP_MAXWIN << tp->rcv_scale.
>                  */
> -               long adv = min(recwin, (long)TCP_MAXWIN << tp->rcv_scale) -
> +               long adv = lmin(recwin, (long)TCP_MAXWIN << tp->rcv_scale) -
>                         (tp->rcv_adv - tp->rcv_nxt);
>  
>                 if(min(recwin, (long)TCP_MAXWIN << tp->rcv_scale) <
> 
> 
> If anyone has an idea what could trigger these cases, I'd be happy to
> help debug. But without a clear testcase, it's a bit difficult.

Honestly, it you can stomach it it might be better to add a KASSERT() and go
ahead and panic and get a coredump.  I think if adv is negative, it's a
consequence of some other bug that we'd rather fix instead.  Having a core
dump where one can examine all the TCP pcb state when rcv_adv is too big is
probably the best way to track that down.

-- 
John Baldwin