Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 9 Sep 2020 11:11:50 +0000 (UTC)
From:      Randall Stewart <rrs@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r365501 - head/sys/netinet/tcp_stacks
Message-ID:  <202009091111.089BBor0001208@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: rrs
Date: Wed Sep  9 11:11:50 2020
New Revision: 365501
URL: https://svnweb.freebsd.org/changeset/base/365501

Log:
  So it turns out that syzkaller hit another crash. It has to do with switching
  stacks with a SENT_FIN outstanding. Both rack and bbr will only send a
  FIN if all data is ack'd so this must be enforced. Also if the previous stack
  sent the FIN we need to make sure in rack that when we manufacture the
  "unknown" sends that we include the proper HAS_FIN bits.
  
  Note for BBR we take a simpler approach and just refuse to switch.
  
  Sponsored by:	Netflix Inc.
  Differential Revision:	https://reviews.freebsd.org/D26269

Modified:
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c

Modified: head/sys/netinet/tcp_stacks/bbr.c
==============================================================================
--- head/sys/netinet/tcp_stacks/bbr.c	Wed Sep  9 09:08:09 2020	(r365500)
+++ head/sys/netinet/tcp_stacks/bbr.c	Wed Sep  9 11:11:50 2020	(r365501)
@@ -10281,6 +10281,8 @@ bbr_handoff_ok(struct tcpcb *tp)
 		 */
 		return (EAGAIN);
 	}
+	if (tp->t_flags & TF_SENTFIN)
+		return (EINVAL);
 	if ((tp->t_flags & TF_SACK_PERMIT) || bbr_sack_not_required) {
 		return (0);
 	}

Modified: head/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack.c	Wed Sep  9 09:08:09 2020	(r365500)
+++ head/sys/netinet/tcp_stacks/rack.c	Wed Sep  9 11:11:50 2020	(r365501)
@@ -10451,7 +10451,12 @@ rack_init(struct tcpcb *tp)
 		rsm->r_rtr_cnt = 1;
 		rsm->r_rtr_bytes = 0;
 		rsm->r_start = tp->snd_una;
-		rsm->r_end = tp->snd_max;
+		if (tp->t_flags & TF_SENTFIN) {
+			rsm->r_end = tp->snd_max - 1;
+			rsm->r_flags |= RACK_HAS_FIN;
+		} else {
+			rsm->r_end = tp->snd_max;
+		}
 		rsm->usec_orig_send = us_cts;
 		rsm->r_dupack = 0;
 		insret = RB_INSERT(rack_rb_tree_head, &rack->r_ctl.rc_mtree, rsm);
@@ -10518,8 +10523,21 @@ rack_handoff_ok(struct tcpcb *tp)
 	if ((tp->t_state == TCPS_SYN_SENT) ||
 	    (tp->t_state == TCPS_SYN_RECEIVED)) {
 		/*
-		 * We really don't know you have to get to ESTAB or beyond
-		 * to tell.
+		 * We really don't know if you support sack, 
+		 * you have to get to ESTAB or beyond to tell.
+		 */
+		return (EAGAIN);
+	}
+	if ((tp->t_flags & TF_SENTFIN) && ((tp->snd_max - tp->snd_una) > 1)) {
+		/*
+		 * Rack will only send a FIN after all data is acknowledged.
+		 * So in this case we have more data outstanding. We can't
+		 * switch stacks until either all data and only the FIN
+		 * is left (in which case rack_init() now knows how
+		 * to deal with that) <or> all is acknowledged and we
+		 * are only left with incoming data, though why you
+		 * would want to switch to rack after all data is acknowledged
+		 * I have no idea (rrs)!
 		 */
 		return (EAGAIN);
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202009091111.089BBor0001208>