netdev - [net PATCH] tcp: Mark fastopen SYN packet as lost when receiving ICMP_TOOBIG/ICMP_FRAG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <160765171921.6905.7897898635812579754.stgit@localhost.localdomain>
Date:   Thu, 10 Dec 2020 17:55:19 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     edumazet@...gle.com, davem@...emloft.net, kuba@...nel.org
Cc:     kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org, kafai@...com,
        kernel-team@...com
Subject: [net PATCH] tcp: Mark fastopen SYN packet as lost when receiving
 ICMP_TOOBIG/ICMP_FRAG_NEEDED

From: Alexander Duyck <alexanderduyck@...com>

In the case of a fastopen SYN there are cases where it may trigger either a
ICMP_TOOBIG message in the case of IPv6 or a fragmentation request in the
case of IPv4. This results in the socket stalling for a second or more as
it does not respond to the message by retransmitting the SYN frame.

Normally a SYN frame should not be able to trigger a ICMP_TOOBIG or
ICMP_FRAG_NEEDED however in the case of fastopen we can have a frame that
makes use of the entire MTU. In the case of fastopen it does, and an
additional complication is that the retransmit queue doesn't contain the
original frames. As a result when tcp_simple_retransmit is called and
walks the list of frames in the queue it may not mark the frames as lost
because both the SYN and the data packet each individually are smaller than
the MSS size after the adjustment. This results in the socket being stalled
until the retransmit timer kicks in and forces the SYN frame out again
without the data attached.

In order to resolve this we need to mark the SYN frame as lost if it is the
first packet in the queue. Doing this allows the socket to recover much
more quickly without the retransmit timeout stall.

Signed-off-by: Alexander Duyck <alexanderduyck@...com>
---
 include/net/tcp.h    |    1 +
 net/ipv4/tcp_input.c |    8 ++++++++
 net/ipv4/tcp_ipv4.c  |    6 ++++++
 net/ipv6/tcp_ipv6.c  |    4 ++++
 4 files changed, 19 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d4ef5bf94168..6181ad98727a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2062,6 +2062,7 @@ void tcp_init(void);
 
 /* tcp_recovery.c */
 void tcp_mark_skb_lost(struct sock *sk, struct sk_buff *skb);
+void tcp_mark_syn_lost(struct sock *sk);
 void tcp_newreno_mark_lost(struct sock *sk, bool snd_una_advanced);
 extern s32 tcp_rack_skb_timeout(struct tcp_sock *tp, struct sk_buff *skb,
 				u32 reo_wnd);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 389d1b340248..d0c5248bc4e1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1066,6 +1066,14 @@ void tcp_mark_skb_lost(struct sock *sk, struct sk_buff *skb)
 	}
 }
 
+void tcp_mark_syn_lost(struct sock *sk)
+{
+	struct sk_buff *skb = tcp_rtx_queue_head(sk);
+
+	if (skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN)
+		tcp_mark_skb_lost(sk, skb);
+}
+
 /* Updates the delivered and delivered_ce counts */
 static void tcp_count_delivered(struct tcp_sock *tp, u32 delivered,
 				bool ece_ack)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 8391aa29e7a4..ad62fe029646 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -546,6 +546,12 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 			if (sk->sk_state == TCP_LISTEN)
 				goto out;
 
+			/* fastopen SYN may have triggered the fragmentation
+			 * request. Mark the SYN or SYN/ACK as lost.
+			 */
+			if (sk->sk_state == TCP_SYN_SENT)
+				tcp_mark_syn_lost(sk);
+
 			tp->mtu_info = info;
 			if (!sock_owned_by_user(sk)) {
 				tcp_v4_mtu_reduced(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 992cbf3eb9e3..d7b1346863e3 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -443,6 +443,10 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		if (!ip6_sk_accept_pmtu(sk))
 			goto out;
 
+		/* fastopen SYN may have triggered TOOBIG, mark it lost. */
+		if (sk->sk_state == TCP_SYN_SENT)
+			tcp_mark_syn_lost(sk);
+
 		tp->mtu_info = ntohl(info);
 		if (!sock_owned_by_user(sk))
 			tcp_v6_mtu_reduced(sk);