[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZS1/qtr0dZJ35VII@debian.debian>
Date: Mon, 16 Oct 2023 11:23:38 -0700
From: Yan Zhai <yan@...udflare.com>
To: netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Aya Levin <ayal@...dia.com>,
Tariq Toukan <tariqt@...dia.com>, linux-kernel@...r.kernel.org,
kernel-team@...udflare.com, Florian Westphal <fw@...len.de>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: [PATCH v2 net-next] ipv6: avoid atomic fragment on GSO packets
GSO packets can contain a trailing segment that is smaller than
gso_size. When examining the dst MTU for such packet, if its gso_size is
too large, then all segments would be fragmented. However, there is a
good chance the trailing segment has smaller actual size than both
gso_size as well as the MTU, which leads to an "atomic fragment". It is
considered harmful in RFC-8021. An Existing report from APNIC also shows
that atomic fragments are more likely to be dropped even it is
equivalent to a no-op [1].
Refactor __ip6_finish_output code to separate GSO and non-GSO packet
processing. It mirrors __ip_finish_output logic now. Add an extra check
in GSO handling to avoid atomic fragments. Lastly, drop dst_allfrag
check, which is no longer true since commit 9d289715eb5c ("ipv6: stop
sending PTB packets for MTU < 1280").
Link: https://www.potaroo.net/presentations/2022-03-01-ipv6-frag.pdf [1]
Fixes: b210de4f8c97 ("net: ipv6: Validate GSO SKB before finish IPv6 processing")
Suggested-by: Florian Westphal <fw@...len.de>
Reported-by: David Wragg <dwragg@...udflare.com>
Signed-off-by: Yan Zhai <yan@...udflare.com>
---
net/ipv6/ip6_output.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a471c7e91761..1de6f3c11655 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -162,7 +162,14 @@ ip6_finish_output_gso_slowpath_drop(struct net *net, struct sock *sk,
int err;
skb_mark_not_on_list(segs);
- err = ip6_fragment(net, sk, segs, ip6_finish_output2);
+ /* Last gso segment might be smaller than actual MTU. Adding
+ * a fragment header to it would produce an "atomic fragment",
+ * which is considered harmful (RFC-8021)
+ */
+ err = segs->len > mtu ?
+ ip6_fragment(net, sk, segs, ip6_finish_output2) :
+ ip6_finish_output2(net, sk, segs);
+
if (err && ret == 0)
ret = err;
}
@@ -170,10 +177,19 @@ ip6_finish_output_gso_slowpath_drop(struct net *net, struct sock *sk,
return ret;
}
+static int ip6_finish_output_gso(struct net *net, struct sock *sk,
+ struct sk_buff *skb, unsigned int mtu)
+{
+ if (!(IP6CB(skb)->flags & IP6SKB_FAKEJUMBO) &&
+ !skb_gso_validate_network_len(skb, mtu))
+ return ip6_finish_output_gso_slowpath_drop(net, sk, skb, mtu);
+
+ return ip6_finish_output2(net, sk, skb);
+}
+
static int __ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
unsigned int mtu;
-
#if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
/* Policy lookup after SNAT yielded a new policy */
if (skb_dst(skb)->xfrm) {
@@ -183,17 +199,14 @@ static int __ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff
#endif
mtu = ip6_skb_dst_mtu(skb);
- if (skb_is_gso(skb) &&
- !(IP6CB(skb)->flags & IP6SKB_FAKEJUMBO) &&
- !skb_gso_validate_network_len(skb, mtu))
- return ip6_finish_output_gso_slowpath_drop(net, sk, skb, mtu);
+ if (skb_is_gso(skb))
+ return ip6_finish_output_gso(net, sk, skb, mtu);
- if ((skb->len > mtu && !skb_is_gso(skb)) ||
- dst_allfrag(skb_dst(skb)) ||
+ if (skb->len > mtu ||
(IP6CB(skb)->frag_max_size && skb->len > IP6CB(skb)->frag_max_size))
return ip6_fragment(net, sk, skb, ip6_finish_output2);
- else
- return ip6_finish_output2(net, sk, skb);
+
+ return ip6_finish_output2(net, sk, skb);
}
static int ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff *skb)
--
2.30.2
Powered by blists - more mailing lists