tcp/sack: Cleanup the SACK related bits when return from tcp_input slow path
authorSepherosa Ziehau <sephe@dragonflybsd.org>
Fri, 6 Apr 2012 01:47:36 +0000 (09:47 +0800)
committerSepherosa Ziehau <sephe@dragonflybsd.org>
Wed, 11 Apr 2012 01:43:09 +0000 (09:43 +0800)
When the code path that could generate the invalid SACK block on out-of-order
FIN segment was walked through, it turned out that some SACK related bits
could be leftover from the previous input segment, e.g. SACKLEFT.

Since the observed invalid SACK option contains only one SACK block and the
block's right edge is same as its left edge, the code which can generate that
SACK block could be run only if SACKLEFT is turned on and reportblk's start
and end are same.
- If SACKLEFT is on then reportblk's start and end must not be same, so the
  SACKLEFT is leftover from the previous input segment.
- If KASSERTs about the clearance of the SACK bits are added immediately
  before returning from the tcp_input slow path, the KASSERTs could fail due
  to SACKLEFT are still on.

Obviously the SACK related bits are never intended to be inheritted from one
input segment to another input segment, so we just clear them before returning
from the tcp_input slow path.

sys/netinet/tcp_input.c
sys/netinet/tcp_sack.c
sys/netinet/tcp_var.h

index da48669..a153484 100644 (file)
@@ -2524,6 +2524,7 @@ dodata:                                                   /* XXX */
         */
        if (needoutput || (tp->t_flags & TF_ACKNOW))
                tcp_output(tp);
+       tcp_sack_report_cleanup(tp);
        return(IPPROTO_DONE);
 
 dropafterack:
@@ -2555,6 +2556,7 @@ dropafterack:
        m_freem(m);
        tp->t_flags |= TF_ACKNOW;
        tcp_output(tp);
+       tcp_sack_report_cleanup(tp);
        return(IPPROTO_DONE);
 
 dropwithreset:
@@ -2601,6 +2603,8 @@ dropwithreset:
                tcp_respond(tp, mtod(m, void *), th, m, th->th_seq + tlen,
                            (tcp_seq)0, TH_RST | TH_ACK);
        }
+       if (tp != NULL)
+               tcp_sack_report_cleanup(tp);
        return(IPPROTO_DONE);
 
 drop:
@@ -2612,6 +2616,8 @@ drop:
                tcp_trace(TA_DROP, ostate, tp, tcp_saveipgen, &tcp_savetcp, 0);
 #endif
        m_freem(m);
+       if (tp != NULL)
+               tcp_sack_report_cleanup(tp);
        return(IPPROTO_DONE);
 }
 
index 765b2e5..0549792 100644 (file)
@@ -204,6 +204,16 @@ tcp_sack_cleanup(struct scoreboard *scb)
 }
 
 /*
+ * Cleanup the reported SACK block information
+ */
+void
+tcp_sack_report_cleanup(struct tcpcb *tp)
+{
+       tp->t_flags &= ~(TF_DUPSEG | TF_ENCLOSESEG | TF_SACKLEFT);
+       tp->reportblk.rblk_start = tp->reportblk.rblk_end;
+}
+
+/*
  * Returns     0 if not D-SACK block,
  *             1 if D-SACK,
  *             2 if duplicate of out-of-order D-SACK block.
index fa1127d..57535d0 100644 (file)
@@ -621,6 +621,7 @@ struct rtentry *
         tcp_rtlookup (struct in_conninfo *);
 int     tcp_sack_bytes_below(struct scoreboard *scb, tcp_seq seq);
 void    tcp_sack_cleanup(struct scoreboard *scb);
+void    tcp_sack_report_cleanup(struct tcpcb *tp);
 int     tcp_sack_ndsack_blocks(struct raw_sackblock *blocks,
            const int numblocks, tcp_seq snd_una);
 void    tcp_sack_fill_report(struct tcpcb *tp, u_char *opt, u_int *plen);