Message-ID: <20240922150450.3873767-1-willemdebruijn.kernel@gmail.com>
Date: Sun, 22 Sep 2024 11:03:45 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net,
	kuba@...nel.org,
	edumazet@...gle.com,
	pabeni@...hat.com,
	stable@...r.kernel.org,
	maze@...gle.com,
	shiming.cheng@...iatek.com,
	daniel@...earbox.net,
	lena.wang@...iatek.com,
	herbert@...dor.apana.org.au,
	Willem de Bruijn <willemb@...gle.com>
Subject: [PATCH net] gso: fix gso fraglist segmentation after pull from frag_list

From: Willem de Bruijn <willemb@...gle.com>

Detect gso fraglist skbs with corrupted geometry (see below) and
pass these to skb_segment instead of skb_segment_list, as the first
can segment them correctly.

Valid SKB_GSO_FRAGLIST skbs
- consist of two or more segments
- the head_skb holds the protocol headers plus first gso_size
- one or more frag_list skbs hold exactly one segment
- all but the last must be gso_size

Optional datapath hooks such as NAT and BPF (bpf_skb_pull_data) can
modify these skbs, breaking these invariants.

In extreme cases they pull all data into skb linear. For UDP, this
causes a NULL ptr deref in __udpv4_gso_segment_list_csum at
udp_hdr(seg->next)->dest.

Detect invalid geometry due to pull, by checking head_skb size.
Don't just drop, as this may blackhole a destination. Convert to be
able to pass to regular skb_segment.

Link: https://lore.kernel.org/netdev/20240428142913.18666-1-shiming.cheng@mediatek.com/
Fixes: 3a1296a38d0c ("net: Support GRO/GSO fraglist chaining.")
Signed-off-by: Willem de Bruijn <willemb@...gle.com>
Cc: stable@...r.kernel.org

---

Tested:
- tools/testing/selftests/net/udpgro_fwd.sh
- kunit gso_test_func converted to calling __udp_gso_segment
- below manual end-to-end test:
  (which probably repeats a lot of udpgro_fwd.sh, in hindsight..)
  (won't repeat this on any resubmits, given how long it is)

  #!/bin/bash

  ip netns add test1
  ip netns add test2
  ip netns add test3

  ip link add dev veth0 netns test1 type veth peer name veth0 netns test2
  ip link add dev veth1 netns test2 type veth peer name veth1 netns test3

  ip netns exec test1 ip link set dev veth0 up
  ip netns exec test2 ip link set dev veth0 up

  ip netns exec test2 ip link set dev veth1 up
  ip netns exec test3 ip link set dev veth1 up

  ip netns exec test1 ip addr add 10.0.8.1/24 dev veth0
  ip netns exec test2 ip addr add 10.0.8.2/24 dev veth0

  ip netns exec test2 ip addr add 10.0.9.2/24 dev veth1
  ip netns exec test3 ip addr add 10.0.9.3/24 dev veth1

  ip -6 -netns test1 addr add fdaa::1 dev veth0
  ip -6 -netns test2 addr add fdaa::2 dev veth0

  ip -6 -netns test2 addr add fdbb::2 dev veth1
  ip -6 -netns test3 addr add fdbb::3 dev veth1

  ip netns exec test2 sysctl -w net.ipv4.ip_forward=1
  ip netns exec test2 sysctl -w net.ipv6.conf.all.forwarding=1

  ip -netns test1 route add default via 10.0.8.2
  ip -netns test3 route add default via 10.0.9.2

  ip -6 -netns test1 route add fdaa::2 dev veth0
  ip -6 -netns test2 route add fdaa::1 dev veth0
  ip -6 -netns test2 route add fdbb::3 dev veth1
  ip -6 -netns test3 route add fdbb::2 dev veth1

  ip -6 -netns test1 route add default via fdaa::2
  ip -6 -netns test3 route add default via fdbb::2

  ip netns exec test1 ethtool -K veth0 gso off tx-udp-segmentation off
  ip netns exec test2 ethtool -K veth0 gro on rx-gro-list on rx-udp-gro-forwarding on
  ip netns exec test2 ethtool -K veth1 gso off tx-udp-segmentation off

  ip netns exec test2 tc qdisc add dev veth0 clsact
  ip netns exec test2 tc filter add dev veth0 ingress bpf direct-action obj tc_pull.o sec tc

  ip netns exec test3 /mnt/shared/udpgso_bench_rx & \
  ip netns exec test1 /mnt/shared/udpgso_bench_tx -l 5 -4 -D 10.0.9.3 -s
  60000 -S 0 -z

  ip netns exec test3 /mnt/shared/udpgso_bench_rx & \
  ip netns exec test1 /mnt/shared/udpgso_bench_tx -l 5 -6 -D fdbb::3 -s
  60000 -S 0 -z

With trivial BPF program:

  $ cat ~/work/tc_pull.c
  // SPDX-License-Identifier: GPL-2.0

  #include <linux/bpf.h>
  #include <linux/pkt_cls.h>
  #include <linux/types.h>

  #include <bpf/bpf_helpers.h>

  __attribute__((section("tc")))

  int tc_cls_prog(struct __sk_buff *skb) {
  	bpf_skb_pull_data(skb, skb->len);
  	return TC_ACT_OK;
  }
---
 net/ipv4/udp_offload.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index d842303587af..e457fa9143a6 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -296,8 +296,16 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 		return NULL;
 	}
 
-	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
-		return __udp_gso_segment_list(gso_skb, features, is_ipv6);
+	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
+		 /* Detect modified geometry and pass these to skb_segment. */
+		if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
+			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
+
+		 /* Setup csum, as fraglist skips this in udp4_gro_receive. */
+		gso_skb->csum_start = skb_transport_header(gso_skb) - gso_skb->head;
+		gso_skb->csum_offset = offsetof(struct udphdr, check);
+		gso_skb->ip_summed = CHECKSUM_PARTIAL;
+	}
 
 	skb_pull(gso_skb, sizeof(*uh));
 
-- 
2.46.0.792.g87dc391469-goog

Powered by blists - more mailing lists