From da7c5efcaba38247f7f69927d692aca3796051c8 Mon Sep 17 00:00:00 2001
From: rrs <rrs@FreeBSD.org>
Date: Wed, 9 Sep 2020 11:11:50 +0000
Subject: [PATCH] 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
---
 sys/netinet/tcp_stacks/bbr.c  |  2 ++
 sys/netinet/tcp_stacks/rack.c | 24 +++++++++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/sys/netinet/tcp_stacks/bbr.c b/sys/netinet/tcp_stacks/bbr.c
index e5d73b6826ff..2e74ecbe8ea1 100644
--- a/sys/netinet/tcp_stacks/bbr.c
+++ b/sys/netinet/tcp_stacks/bbr.c
@@ -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);
 	}
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index e18528a4463d..635e932bb80e 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -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);
 	}