http://perforce.freebsd.org/chv.cgi?CH=138288
Change 138288 by andre at andre_flirtbox on 2008/03/22 16:30:22
Simplified FIN handling based on discussion on TCPM mailing
list.
Affected files ...
.. //depot/projects/tcp_reass/netinet/tcp_reass.c#19 edit
Differences ...
==== //depot/projects/tcp_reass/netinet/tcp_reass.c#19 (text+ko) ====
@@ -286,15 +286,29 @@
mcnt += (n->m_flags & M_EXT) ?
n->m_ext.ext_size + MSIZE : MSIZE;
- tqe = TAILQ_LAST(&tp->t_trq, trq_head);
-
/*
* FIN handling is a bit tricky.
- * We only accept a FIN if it matches the right side of the sequence
- * space.
+ * We cannot trust a FIN that goes into the reassembly queue.
+ * It can be easily spoofed as it may be anywhere in the receive
+ * window (see RST attack mitigation in tcp-secure).
+ * For this reason (and complexity avoidance) we generally ignore
+ * any FIN arriving at the reassembly queue with one exception;
+ * When it exactly matches rcv_nxt together with any data in the
+ * same segment we can conclude it to be genuine and proceed with
+ * flushing any other data waiting in the reassembly queue.
+ * A FIN is part of the sequence space and will get retransmitted
+ * if it was genuine.
+ * This approach is based on a discussion on TCPM mailing list.
*/
- if (thflags & TH_FIN) {
- }
+ if ((thflags & TH_FIN) && tp->rcv_nxt == th_seq) {
+ tcp_reass_qfree(tp);
+ tqe = NULL;
+ goto insert;
+ } else
+ thflags &= ~TH_FIN;
+
+ /* Starting point for the following tests. */
+ tqe = TAILQ_LAST(&tp->t_trq, trq_head);
/* Check if this segment directly attaches to the end. */
if (tqe && tqe->trq_seq + tqe->trq_len == th_seq) {
@@ -525,7 +539,7 @@
return (0);
present:
/*
- * Present data to user, advancing rcv_nxt through
+ * Present data to user, advancing rcv_nxt through the
* completed sequence space.
*/
KASSERT(!TAILQ_EMPTY(&tp->t_trq),
@@ -533,6 +547,7 @@
KASSERT((TAILQ_FIRST(&tp->t_trq))->trq_seq == tp->rcv_nxt,
("%s: first block does not match rcv_nxt", __func__));
tcpstat.tcps_reass_missingseg++;
+
SOCKBUF_LOCK(&so->so_rcv);
TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt),
@@ -540,6 +555,7 @@
KASSERT(tqen == NULL ||
SEQ_LEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq),
("%s: block overlaps into next one", __func__));
+
if (tqe->trq_seq != tp->rcv_nxt)
break;
if (so->so_rcv.sb_state & SBS_CANTRCVMORE)
@@ -547,8 +563,6 @@
else
sbappendstream_locked(&so->so_rcv, tqe->trq_m);
tp->rcv_nxt += tqe->trq_len;
- KASSERT(!(thflags & TH_FIN) || tqe == TAILQ_LAST(&tp->t_trq, trq_head),
- ("%s: FIN not on last block", __func__));
tp->t_trqmcnt -= tqe->trq_mcnt;
tcp_reass_mcnt -= tqe->trq_mcnt;
TAILQ_REMOVE(&tp->t_trq, tqe, trq_q);
@@ -557,7 +571,7 @@
uma_zfree(tcp_reass_zone, tqe);
tcp_reass_qsize--;
}
- /* NB: sorwakeup_locked() does an implicit socket buffer unlock. */
+ /* NB: sorwakeup_locked() does a implicit socket buffer unlock. */
sorwakeup_locked(so);
/*