Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Oct 2023 19:19:42 GMT
From:      Randall Stewart <rrs@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 8818f0f1124e - main - TCP: Fix a rack bug that skyzall found which results in a crash.
Message-ID:  <202310041919.394JJgUE073149@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by rrs:

URL: https://cgit.FreeBSD.org/src/commit/?id=8818f0f1124ea3d0e8028f85d667237536eba10c

commit 8818f0f1124ea3d0e8028f85d667237536eba10c
Author:     Randall Stewart <rrs@FreeBSD.org>
AuthorDate: 2023-10-04 19:16:01 +0000
Commit:     Randall Stewart <rrs@FreeBSD.org>
CommitDate: 2023-10-04 19:16:01 +0000

    TCP: Fix a rack bug that skyzall found which results in a crash.
    
    So when we call the fast_rsm retransmit path, we should always move
    snd_nxt back up to snd_max. In fact during ack-processing if snd_nxt
    falls behind it should be moved up there as well. Otherwise what
    can happen is we have an incorrect mark on snd_nxt and incorrectly
    calculate the offset when we go through the  front path (which is
    what skzyall was able to do) then when we go to clean up the
    send the offset is all wrong and we crash.
    
    Special thanks to Gleb for pointing out the problem and the email
    that had the reproducer so I could find the issue.
    
    Reported-by: syzbot+f5061a372f74f021ec02@syzkaller.appspotmail.com
    Sponsored by: Netflix Inc
---
 sys/netinet/tcp_stacks/rack.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index a6e649f78c10..5c16cb9ed23b 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -12342,8 +12342,8 @@ rack_process_ack(struct mbuf *m, struct tcphdr *th, struct socket *so,
 	if (SEQ_GT(tp->snd_una, tp->snd_recover))
 		tp->snd_recover = tp->snd_una;
 
-	if (SEQ_LT(tp->snd_nxt, tp->snd_una)) {
-		tp->snd_nxt = tp->snd_una;
+	if (SEQ_LT(tp->snd_nxt, tp->snd_max)) {
+		tp->snd_nxt = tp->snd_max;
 	}
 	if (under_pacing &&
 	    (rack->use_fixed_rate == 0) &&
@@ -16363,8 +16363,8 @@ rack_do_compressed_ack_processing(struct tcpcb *tp, struct socket *so, struct mb
 		/* Send recover and snd_nxt must be dragged along */
 		if (SEQ_GT(tp->snd_una, tp->snd_recover))
 			tp->snd_recover = tp->snd_una;
-		if (SEQ_LT(tp->snd_nxt, tp->snd_una))
-			tp->snd_nxt = tp->snd_una;
+		if (SEQ_LT(tp->snd_nxt, tp->snd_max))
+			tp->snd_nxt = tp->snd_max;
 		/*
 		 * If the RXT timer is running we want to
 		 * stop it, so we can restart a TLP (or new RXT).
@@ -19112,6 +19112,8 @@ rack_fast_rsm_output(struct tcpcb *tp, struct tcp_rack *rack, struct rack_sendma
 		lgb->tlb_errno = error;
 		lgb = NULL;
 	}
+	/* Move snd_nxt to snd_max so we don't have false retransmissions */
+	tp->snd_nxt = tp->snd_max;
 	if (error) {
 		goto failed;
 	} else if (rack->rc_hw_nobuf && (ip_sendflag != IP_NO_SND_TAG_RL)) {



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