From patchwork Wed Apr 17 16:57:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 13633645 X-Patchwork-Delegate: kuba@kernel.org Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E96B7171667 for ; Wed, 17 Apr 2024 16:58:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713373082; cv=none; b=FaS9aKYLBBPvgTFLIXiid/DoxPmjUEQ47KCO34KA8BrA3R1lAuKvyaCmfd7JwAcuXIZHcOAIF2GssA+momiT9WBY0PYEX9bW5CAPiDZRw9ddm4lLsPJg1Nd9nOoblinvEgdxV+t7wZLyiAiKQTmiWhLgJuuVLbITJdDRYxsjIhs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713373082; c=relaxed/simple; bh=DrlNDSy3+s1STa9+cKOwIzy5XOiKyi3sfi5Nz7PQ98s=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=Lm6Ra02paHApo68BxYUT9pgfZS4TRL0Cw853qPKYR+rvLGoaW31AeNVK7s4xtueJH+d0Mz2SgUUbymvBM+BzRKTHqrKQKr5AXQzxyidsDeprIn9HApMnSwfXXIjb5I4ZZqj7y4xuj/CWPoWAu9dvrsgNHYdIFVEg8KYVmi7CRf0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--edumazet.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=sTmJsoAq; arc=none smtp.client-ip=209.85.219.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--edumazet.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="sTmJsoAq" Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-dc6b269686aso8248335276.1 for ; Wed, 17 Apr 2024 09:58:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713373080; x=1713977880; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=39SES1xI7t7XpswHO/i5Yy26SxvrbXiOwaJYlGSFJHo=; b=sTmJsoAqUsOCr1co0Tk8N9vW1iKp1SywVezjCP3dh2vk7JLj6M6VVIGeTrJ/4bkLEZ Ix6z+XrqFgSAIxq4iOf07QLUJ7Ov8SMIH2Og4wVnva9ZaRmignT2VJKp/ZbsQebelCSy 8d4WJ1mMD9BlH2YWjcyYxxJZWo8lm2uKrcC+ur+kIIIH6Ig1Sxp4MIOSY9oBl4Q95ZMg 5dC6tI/2uqZCZK5aumQo/SFJDwjK284+gPRqj2CpkfYFiYMbsDBfyAvQ2T0AdNBISSS1 GUy0uZ+NRsZq4wjzN3AC9QMMfBom9I+03GT1ctf5Uuq42MKOQwKiH+Ggouk63dyIuCxT 8eQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713373080; x=1713977880; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=39SES1xI7t7XpswHO/i5Yy26SxvrbXiOwaJYlGSFJHo=; b=YjSNQPmeImYyOzArOmmgdT/xRkgA/gfhJhGEBYmFNqH7Ges4fsJWmK/yyciAlXt/g7 YXu6cprIrqNJuLUob/xA7UfuOVFlSao1Fr5QbH+gQPaQGq2j3TDHu9oqjKATTd5N07FL 9AkZ4e94LFm8B/M/1AYGQNm+SI8OOoWa69lLseWssLbassDpDRJw8Jp3Trc77GXhwk5Y 2bzoan1+rc6uBWOWZlYj79QU6qcpJjnoK19s06o/RZr0Xj0jw2HLzPmUHrWA8RYlbSTh 7dJM3CWDntHJVEe3fBwZN5koQVV+5n2HeLIiJTNdUolh7Twqjk5eWQedWkQEcSqpGNiE jAEg== X-Gm-Message-State: AOJu0Yx+XQcPgCB2T7TrAB8z5avd8lNPG4pe75zuadNOxLCMgp9drBE4 AqvHGyUMFXCRl8hkbbEXnBK8BTzdHJx1B9RpwWRrdo6XmB1lz9fXi4nbSzw3V73+Si4fxyMTutH hHbQ+uiOxIQ== X-Google-Smtp-Source: AGHT+IHzl2YEfeLQ1dJ6yvhTKl8t5nQ9FBNDRFAg1i9UMp40Ehn2ti/eBrKJDtOrU4K3C83yw34gAnB2CETSEg== X-Received: from edumazet1.c.googlers.com ([fda3:e722:ac3:cc00:2b:7d90:c0a8:395a]) (user=edumazet job=sendgmr) by 2002:a25:ac4c:0:b0:dcd:c091:e86 with SMTP id r12-20020a25ac4c000000b00dcdc0910e86mr1190935ybd.13.1713373079814; Wed, 17 Apr 2024 09:57:59 -0700 (PDT) Date: Wed, 17 Apr 2024 16:57:55 +0000 In-Reply-To: <20240417165756.2531620-1-edumazet@google.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240417165756.2531620-1-edumazet@google.com> X-Mailer: git-send-email 2.44.0.683.g7961c838ac-goog Message-ID: <20240417165756.2531620-2-edumazet@google.com> Subject: [PATCH net-next 1/2] tcp: conditionally call ip_icmp_error() from tcp_v4_err() From: Eric Dumazet To: "David S . Miller" , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, Neal Cardwell , Dragos Tatulea , eric.dumazet@gmail.com, Eric Dumazet , " =?utf-8?q?Maciej_=C5=BBenczykowski?= " , Willem de Bruijn , Shachar Kagan X-Patchwork-Delegate: kuba@kernel.org Blamed commit claimed in its changelog that the new functionality was guarded by IP_RECVERR/IPV6_RECVERR : Note that applications need to set IP_RECVERR/IPV6_RECVERR option to enable this feature, and that the error message is only queued while in SYN_SNT state. This was true only for IPv6, because ipv6_icmp_error() has the following check: if (!inet6_test_bit(RECVERR6, sk)) return; Other callers check IP_RECVERR by themselves, it is unclear if we could factorize these checks in ip_icmp_error() For stable backports, I chose to add the missing check in tcp_v4_err() We think this missing check was the root cause for commit 0a8de364ff7a ("tcp: no longer abort SYN_SENT when receiving some ICMP") breakage, leading to a revert. Many thanks to Dragos Tatulea for conducting the investigations. As Jakub said : The suspicion is that SSH sees the ICMP report on the socket error queue and tries to connect() again, but due to the patch the socket isn't disconnected, so it gets EALREADY, and throws its hands up... The error bubbles up to Vagrant which also becomes unhappy. Can we skip the call to ip_icmp_error() for non-fatal ICMP errors? Fixes: 45af29ca761c ("tcp: allow traceroute -Mtcp for unpriv users") Signed-off-by: Eric Dumazet Tested-by: Dragos Tatulea Cc: Dragos Tatulea Cc: Maciej Żenczykowski Cc: Willem de Bruijn Cc: Neal Cardwell Cc: Shachar Kagan Reviewed-by: Maciej Żenczykowski Reviewed-by: Jason Xing --- net/ipv4/tcp_ipv4.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 88c83ac4212957f19efad0f967952d2502bdbc7f..a717db99972d977a64178d7ed1109325d64a6d51 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -602,7 +602,8 @@ int tcp_v4_err(struct sk_buff *skb, u32 info) if (fastopen && !fastopen->sk) break; - ip_icmp_error(sk, skb, err, th->dest, info, (u8 *)th); + if (inet_test_bit(RECVERR, sk)) + ip_icmp_error(sk, skb, err, th->dest, info, (u8 *)th); if (!sock_owned_by_user(sk)) { WRITE_ONCE(sk->sk_err, err); From patchwork Wed Apr 17 16:57:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 13633646 X-Patchwork-Delegate: kuba@kernel.org Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 612C8171657 for ; Wed, 17 Apr 2024 16:58:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713373083; cv=none; b=L8lhDHmeWf6TXp9Ce7FrA3+31tODrnFYoeht1uU68Zrt0v8eEzCUVKbU5Ubez7SU6kxOvMhbZ1zIoNF2M66Dcw8jsPO+V8v0JaDES0LGsH20BHxAiXvi970RLCWPWQ7RO4jwplMWp4JOY1Mp0s+Tzcils++ARmoOyi3eTMsAPQA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713373083; c=relaxed/simple; bh=N2jmBhMBn5+m4fm5zFW8foolZM+iM6Bte4t0VVhlVmw=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=q17dzAKXrSaOuaIdpKPqCavCtjX0OPAP/sVjvNYdvCgRMu2lULz457r0Km0jyjutnCswypzdAr0gDBdABI8v7aWjWq9zK6YdbcPKdOZQ+3D4a3Ik4NnMc/xCDS0znA8SXNjvOVaIVHsyJyE+ahN8Dfm+19CWKQ6X6tuhrJF1EBU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--edumazet.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=JX8RjuQ7; arc=none smtp.client-ip=209.85.219.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--edumazet.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="JX8RjuQ7" Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-de45d8ec875so419828276.3 for ; Wed, 17 Apr 2024 09:58:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713373081; x=1713977881; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=9SfHsMjvbsZ8saPJkjIJdYMKlU+OKfppDgN6rcyAi7Y=; b=JX8RjuQ7mG9zOrFZYtiS4U+EYp7st2FfajGj2AbXZwQRgMRAsDKL07aTfcDM2YPIyJ skxSb/hA+t0oO+GK03F9aODPFFSuSZjBh9vJvwzCpr3xMvJmKVL2IDnauCdMny3iU/qh ePCL5aunD3hIb6YdYK/Bp9zsVnkAZlf+f5ybASg5bBMh6FsBMtuY5dyutSocYFVP+Q1m yJ8waG/J2IHkhXyyb7Tj7oeFYcLs/86YtJidxlFPpE2W9xmGXyFB4PmyJjv6oXE1oScs cpyX5UkMENwTC0ij3N8mNUR6P1VybzOIu49Rrub0/nJX993oTs44w6dysE/8gtNxUZ61 NtKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713373081; x=1713977881; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9SfHsMjvbsZ8saPJkjIJdYMKlU+OKfppDgN6rcyAi7Y=; b=Yn9zrH6Q1rcm+jS5POFAvd8w6Q6w/w6LC/cIJ87eMAX/348sY9GWhj4xGw/eZzeVux jBL9Eq57Q1UEN0gZWvus/NfVQ0dTxgJEM/Bll/Z6ObVOzDQ21BbuYFrIeBHjFyQ/HDzL noMDQfCwR2sGAnY5P5b7j52N3wyFGIX7VRZc+l3wTo4xuWVPTE/sgN70UU20WSUuaBGE X6gVXs7Zwaz8LlVVrmM8vQ1ve9no8tpt3sX2sSZUAKPnl6zajP7hdKqAngBlabDpvlC0 5w9BgyVnh8TVTzPBhmqUcQOL49/fG3XbeUKVhDqhKv4ayYyoZ/UUnSoMv63u6F3Xqhii 2QCw== X-Gm-Message-State: AOJu0Yx0Ee7pPEEEpdC+lW9m7/0lzzE9+8pVZ9QBCYR/9+d9NxwnVGRj FthmQYCgMmpbpU16nDss4b5CWaxUNM3BufAUfd95YQbmmPrcmuuEVlclxX04I8tYXEKMjK9sqU8 R60iCP5Gcfg== X-Google-Smtp-Source: AGHT+IEg1v6If1wu5z2wGkUiaGBXXjWDiofPW7ixTY7LVjpINF01obAWlwjXMefuIFNEecyyMeVQVHMX/iIBsg== X-Received: from edumazet1.c.googlers.com ([fda3:e722:ac3:cc00:2b:7d90:c0a8:395a]) (user=edumazet job=sendgmr) by 2002:a05:6902:2b8b:b0:dc6:fa35:b42 with SMTP id fj11-20020a0569022b8b00b00dc6fa350b42mr5290472ybb.2.1713373081352; Wed, 17 Apr 2024 09:58:01 -0700 (PDT) Date: Wed, 17 Apr 2024 16:57:56 +0000 In-Reply-To: <20240417165756.2531620-1-edumazet@google.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240417165756.2531620-1-edumazet@google.com> X-Mailer: git-send-email 2.44.0.683.g7961c838ac-goog Message-ID: <20240417165756.2531620-3-edumazet@google.com> Subject: [PATCH net-next 2/2] tcp: no longer abort SYN_SENT when receiving some ICMP (II) From: Eric Dumazet To: "David S . Miller" , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, Neal Cardwell , Dragos Tatulea , eric.dumazet@gmail.com, Eric Dumazet X-Patchwork-Delegate: kuba@kernel.org Notes: - A prior version of this patch in commit 0a8de364ff7a ("tcp: no longer abort SYN_SENT when receiving some ICMP") had to be reverted. - We found the root cause, and fixed it in prior patch in the series. - Many thanks to Dragos Tatulea ! Currently, non fatal ICMP messages received on behalf of SYN_SENT sockets do call tcp_ld_RTO_revert() to implement RFC 6069, but immediately call tcp_done(), thus aborting the connect() attempt. This violates RFC 1122 following requirement: 4.2.3.9 ICMP Messages ... o Destination Unreachable -- codes 0, 1, 5 Since these Unreachable messages indicate soft error conditions, TCP MUST NOT abort the connection, and it SHOULD make the information available to the application. This patch makes sure non 'fatal' ICMP[v6] messages do not abort the connection attempt. It enables RFC 6069 for SYN_SENT sockets as a result. Signed-off-by: Eric Dumazet Cc: Neal Cardwell Tested-by: Dragos Tatulea Reviewed-by: Jason Xing --- net/ipv4/tcp_ipv4.c | 6 ++++++ net/ipv6/tcp_ipv6.c | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index a717db99972d977a64178d7ed1109325d64a6d51..4b50d09f84b9ae7fec98a71bedf39594ab85e5ea 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -482,6 +482,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info) const int code = icmp_hdr(skb)->code; struct sock *sk; struct request_sock *fastopen; + bool harderr = false; u32 seq, snd_una; int err; struct net *net = dev_net(skb->dev); @@ -555,6 +556,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info) goto out; case ICMP_PARAMETERPROB: err = EPROTO; + harderr = true; break; case ICMP_DEST_UNREACH: if (code > NR_ICMP_UNREACH) @@ -579,6 +581,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info) } err = icmp_err_convert[code].errno; + harderr = icmp_err_convert[code].fatal; /* check if this ICMP message allows revert of backoff. * (see RFC 6069) */ @@ -605,6 +608,9 @@ int tcp_v4_err(struct sk_buff *skb, u32 info) if (inet_test_bit(RECVERR, sk)) ip_icmp_error(sk, skb, err, th->dest, info, (u8 *)th); + if (!harderr) + break; + if (!sock_owned_by_user(sk)) { WRITE_ONCE(sk->sk_err, err); diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index bb7c3caf4f8536dabdcb3dbe7c90aff9c8985c90..f31527f0a13dee9488ce72834d89524e83f61e5f 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -382,7 +382,7 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt, struct tcp_sock *tp; __u32 seq, snd_una; struct sock *sk; - bool fatal; + bool harderr; int err; sk = __inet6_lookup_established(net, net->ipv4.tcp_death_row.hashinfo, @@ -403,9 +403,9 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt, return 0; } seq = ntohl(th->seq); - fatal = icmpv6_err_convert(type, code, &err); + harderr = icmpv6_err_convert(type, code, &err); if (sk->sk_state == TCP_NEW_SYN_RECV) { - tcp_req_err(sk, seq, fatal); + tcp_req_err(sk, seq, harderr); return 0; } @@ -490,6 +490,9 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt, ipv6_icmp_error(sk, skb, err, th->dest, ntohl(info), (u8 *)th); + if (!harderr) + break; + if (!sock_owned_by_user(sk)) { WRITE_ONCE(sk->sk_err, err); sk_error_report(sk); /* Wake people up to see the error (see connect in sock.c) */