net: tcp: Enforce reasonable 'acked' values
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 19 Sep 2017 18:56:07 +0000 (14:56 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 20 Sep 2017 21:40:47 +0000 (17:40 -0400)
If the distant end ACKs something outside the window, specifically an ACK
of 0 or less than una (perhaps out-of-order packets?), then 'acked' would
be negative.  That throws off everything, especially the qdiscard() call.
Then if qdiscard() does discard all of 'acked' (which it won't for negative
or very large 'acked'), it'll decrement flgcnt.  That will lead to us
sending FINs and advancing our seq by MTU, though the packets themselves
will have little data (unless we wanted to send something).

In short, madness.

I noticed this when a Layer 1 problem was causing a lot of packet loss.
It'd be nice to run Akaros on NS or some other simulator and see what
happens.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/net/tcp.c

index ba8f353..81d296b 100644 (file)
@@ -2345,11 +2345,8 @@ void update(struct conv *s, Tcp * seg)
        tpriv = s->p->priv;
        tcb = (Tcpctl *) s->ptcl;
 
-       /* if everything has been acked, force output(?) */
-       if (seq_gt(seg->ack, tcb->snd.nxt)) {
-               tcb->flags |= FORCE;
+       if (!seq_within(seg->ack, tcb->snd.una, tcb->snd.nxt))
                return;
-       }
 
        acked = seg->ack - tcb->snd.una;
        tcb->snd.una = seg->ack;
@@ -2462,8 +2459,14 @@ void update(struct conv *s, Tcp * seg)
        }
 
 done:
-       if (qdiscard(s->wq, acked) < acked)
+       if (qdiscard(s->wq, acked) < acked) {
                tcb->flgcnt--;
+               /* This happened due to another bug where acked was very large
+                * (negative), which was interpreted as "hey, one less flag, since they
+                * acked one of our flags (like a SYN).  If flgcnt goes negative,
+                * get_xmit_segment() will attempt to send out large packets. */
+               assert(tcb->flgcnt >= 0);
+       }
 
        if (seq_gt(seg->ack, tcb->snd.urg))
                tcb->snd.urg = seg->ack;