From owner-svn-src-head@freebsd.org Wed Sep 9 11:11:51 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 891E83CCAD9; Wed, 9 Sep 2020 11:11:51 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BmfVH387Jz45GD; Wed, 9 Sep 2020 11:11:51 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 379BC20A6E; Wed, 9 Sep 2020 11:11:51 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 089BBpAr001210; Wed, 9 Sep 2020 11:11:51 GMT (envelope-from rrs@FreeBSD.org) Received: (from rrs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 089BBor0001208; Wed, 9 Sep 2020 11:11:50 GMT (envelope-from rrs@FreeBSD.org) Message-Id: <202009091111.089BBor0001208@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: rrs set sender to rrs@FreeBSD.org using -f From: Randall Stewart Date: Wed, 9 Sep 2020 11:11:50 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r365501 - head/sys/netinet/tcp_stacks X-SVN-Group: head X-SVN-Commit-Author: rrs X-SVN-Commit-Paths: head/sys/netinet/tcp_stacks X-SVN-Commit-Revision: 365501 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Sep 2020 11:11:51 -0000 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) 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); }