The tale of a TCP bug

[John Baldwin]

On Thursday, March 24, 2011 7:02:35 pm Stefan `Sec` Zehl wrote:
> Hi,
> 
> I just subscribed to this list, so sorry if I missed some previous
> discussion on this.
> 
> On Thu, Mar 24, 2011 at 16:15 -0400, John Baldwin wrote:
> [...]
> >                         Otherwise, something like this may apply instead:
> > 
> > Index: tcp_input.c
> > ===================================================================
> > --- tcp_input.c	(revision 219911)
> > +++ tcp_input.c	(working copy)
> > @@ -1694,7 +1694,10 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th,
> >  	win = sbspace(&so->so_rcv);
> >  	if (win < 0)
> >  		win = 0;
> > -	tp->rcv_wnd = imax(win, (int)(tp->rcv_adv - tp->rcv_nxt));
> > +	if (SEQ_GEQ(tp->rcv_adv, tp->rcv_nxt))
> > +		tp->rcv_wnd = imax(win, (int)(tp->rcv_adv - tp->rcv_nxt));
> > +	else
> > +		tp->rcv_wnd = win;
> >  
> >  	/* Reset receive buffer auto scaling when not in bulk receive mode. */
> >  	tp->rfbuf_ts = 0;
> > 
> > I think that will fix tp->rcv_wnd to be correct in this case thus fixing
> > further uses of it.
> 
> I just quickly tested it on my bug scenario,  and it still generates
> adv=-1 in tcp_output
> 
> That is because win=65536, which is bigger than the actually advertised
> window (65535, the max that can be advertised without window scaling).

Ah, ok.  Can you try this patch first (without the other)?  If it doesn't
work then we can refine the patch above further.

Index: tcp_output.c
===================================================================
--- tcp_output.c	(revision 215582)
+++ tcp_output.c	(working copy)
@@ -928,7 +928,8 @@
 	if (recwin < (long)(so->so_rcv.sb_hiwat / 4) &&
 	    recwin < (long)tp->t_maxseg)
 		recwin = 0;
-	if (recwin < (long)(tp->rcv_adv - tp->rcv_nxt))
+	if (SEQ_GT(tp->rcv_adv, tp->rcv_nxt) &&
+	    recwin < (long)(tp->rcv_adv - tp->rcv_nxt))
 		recwin = (long)(tp->rcv_adv - tp->rcv_nxt);
 	if (recwin > (long)TCP_MAXWIN << tp->rcv_scale)
 		recwin = (long)TCP_MAXWIN << tp->rcv_scale;


-- 
John Baldwin