svn commit: r361751 - in head/sys/netinet: . tcp_stacks

[Randall Stewart]

Author: rrs
Date: Wed Jun  3 14:07:31 2020
New Revision: 361751
URL: https://svnweb.freebsd.org/changeset/base/361751

Log:
  This fixes a couple of skyzaller crashes. Most
  of them have to do with TFO. Even the default stack
  had one of the issues:
  
  1) We need to make sure for rack that we don't advance
     snd_nxt beyond iss when we are not doing fast open. We
     otherwise can get a bunch of SYN's sent out incorrectly
     with the seq number advancing.
  2) When we complete the 3-way handshake we should not ever
     append to reassembly if the tlen is 0, if TFO is enabled
     prior to this fix we could still call the reasemmbly. Note
     this effects all three stacks.
  3) Rack like its cousin BBR should track if a SYN is on a
     send map entry.
  4) Both bbr and rack need to only consider len incremented on a SYN
     if the starting seq is iss, otherwise we don't increment len which
     may mean we return without adding a sendmap entry.
  
  This work was done in collaberation with Michael Tuexen, thanks for
  all the testing!
  Sponsored by:	Netflix Inc
  Differential Revision:	https://reviews.freebsd.org/D25000

Modified:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c
  head/sys/netinet/tcp_stacks/tcp_rack.h

Modified: head/sys/netinet/tcp_input.c
==============================================================================
--- head/sys/netinet/tcp_input.c	Wed Jun  3 13:51:53 2020	(r361750)
+++ head/sys/netinet/tcp_input.c	Wed Jun  3 14:07:31 2020	(r361751)
@@ -2989,7 +2989,7 @@ dodata:							/* XXX */
 	 */
 	tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
 		   IS_FASTOPEN(tp->t_flags));
-	if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
+	if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
 	    TCPS_HAVERCVDFIN(tp->t_state) == 0) {
 		tcp_seq save_start = th->th_seq;
 		tcp_seq save_rnxt  = tp->rcv_nxt;

Modified: head/sys/netinet/tcp_stacks/bbr.c
==============================================================================
--- head/sys/netinet/tcp_stacks/bbr.c	Wed Jun  3 13:51:53 2020	(r361750)
+++ head/sys/netinet/tcp_stacks/bbr.c	Wed Jun  3 14:07:31 2020	(r361751)
@@ -6028,7 +6028,7 @@ bbr_log_output(struct tcp_bbr *bbr, struct tcpcb *tp, 
 		 * or FIN if seq_out is adding more on and a FIN is present
 		 * (and we are not resending).
 		 */
-		if (th_flags & TH_SYN)
+		if ((th_flags & TH_SYN) && (tp->iss == seq_out))
 			len++;
 		if (th_flags & TH_FIN)
 			len++;
@@ -8369,7 +8369,7 @@ bbr_process_data(struct mbuf *m, struct tcphdr *th, st
 	 */
 	tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
 		   IS_FASTOPEN(tp->t_flags));
-	if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
+	if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
 	    TCPS_HAVERCVDFIN(tp->t_state) == 0) {
 		tcp_seq save_start = th->th_seq;
 		tcp_seq save_rnxt  = tp->rcv_nxt;

Modified: head/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack.c	Wed Jun  3 13:51:53 2020	(r361750)
+++ head/sys/netinet/tcp_stacks/rack.c	Wed Jun  3 14:07:31 2020	(r361751)
@@ -6237,7 +6237,7 @@ rack_log_output(struct tcpcb *tp, struct tcpopt *to, i
 		 * or FIN if seq_out is adding more on and a FIN is present
 		 * (and we are not resending).
 		 */
-		if (th_flags & TH_SYN)
+		if ((th_flags & TH_SYN) && (seq_out == tp->iss)) 
 			len++;
 		if (th_flags & TH_FIN)
 			len++;
@@ -6280,6 +6280,7 @@ again:
 		rsm->usec_orig_send = us_cts;
 		if (th_flags & TH_SYN) {
 			/* The data space is one beyond snd_una */
+			rsm->r_flags |= RACK_HAS_SIN;
 			rsm->r_start = seq_out + 1;
 			rsm->r_end = rsm->r_start + (len - 1);
 		} else {
@@ -8724,7 +8725,7 @@ rack_process_data(struct mbuf *m, struct tcphdr *th, s
 	 */
 	tfo_syn = ((tp->t_state == TCPS_SYN_RECEIVED) &&
 		   IS_FASTOPEN(tp->t_flags));
-	if ((tlen || (thflags & TH_FIN) || tfo_syn) &&
+	if ((tlen || (thflags & TH_FIN) || (tfo_syn && tlen > 0)) &&
 	    TCPS_HAVERCVDFIN(tp->t_state) == 0) {
 		tcp_seq save_start = th->th_seq;
 		tcp_seq save_rnxt  = tp->rcv_nxt;
@@ -12563,8 +12564,10 @@ again:
 		len = 0;
 	}
 	/* Without fast-open there should never be data sent on a SYN */
-	if ((flags & TH_SYN) && (!IS_FASTOPEN(tp->t_flags)))
+	if ((flags & TH_SYN) && (!IS_FASTOPEN(tp->t_flags))) {
+		tp->snd_nxt = tp->iss;
 		len = 0;
+	}
 	orig_len = len;
 	if (len <= 0) {
 		/*

Modified: head/sys/netinet/tcp_stacks/tcp_rack.h
==============================================================================
--- head/sys/netinet/tcp_stacks/tcp_rack.h	Wed Jun  3 13:51:53 2020	(r361750)
+++ head/sys/netinet/tcp_stacks/tcp_rack.h	Wed Jun  3 14:07:31 2020	(r361751)
@@ -39,6 +39,7 @@
 #define RACK_RWND_COLLAPSED 0x0100/* The peer collapsed the rwnd on the segment */
 #define RACK_APP_LIMITED    0x0200/* We went app limited after this send */
 #define RACK_WAS_ACKED	    0x0400/* a RTO undid the ack, but it already had a rtt calc done */
+#define RACK_HAS_SIN	    0x0800/* SIN is on this guy */
 #define RACK_NUM_OF_RETRANS 3
 
 #define RACK_INITIAL_RTO 1000 /* 1 second in milli seconds */