diff mbox series

[net,2/4] udp: fix out-of-bound at segmentation time

Message ID 5e72b33838c6f19d770062e736f0517610ada4e7.1620223174.git.pabeni@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series udp: more FRAGLIST fixes | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: yoshfuji@linux-ipv6.org dsahern@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 8 this patch: 10
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 77 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 8 this patch: 10
netdev/header_inline success Link

Commit Message

Paolo Abeni May 5, 2021, 3:35 p.m. UTC
In the following scenario:

GRO -> SKB_GSO_FRAGLIST aggregation -> forward ->
  xmit over UDP tunnel -> segmentation

__udp_gso_segment_list() will take place and later
skb_udp_tunnel_segment() will try to make the segmented
packets outer UDP header checksum via gso_make_checksum().

The latter expect valids SKB_GSO_CB(skb)->csum and
SKB_GSO_CB(skb)->csum_start, but such fields are not
initialized by __udp_gso_segment_list().

gso_make_checksum() will end-up using a negative offset and
that will trigger the following splat:

 ==================================================================
 BUG: KASAN: slab-out-of-bounds in do_csum+0x3d8/0x400
 Read of size 1 at addr ffff888113ab5880 by task napi/br_port-81/1105

 CPU: 1 PID: 1105 Comm: napi/br_port-81 Not tainted 5.12.0-rc2.mptcp_autotune_ce84e1323bebe+ #268
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014
 Call Trace:
  dump_stack+0xfa/0x151
  print_address_description.constprop.0+0x16/0xa0
  __kasan_report.cold+0x37/0x80
  kasan_report+0x3a/0x50
  do_csum+0x3d8/0x400
  csum_partial+0x21/0x30
  __skb_udp_tunnel_segment+0xd79/0x1ae0
  skb_udp_tunnel_segment+0x233/0x460
  udp4_ufo_fragment+0x50d/0x720
  inet_gso_segment+0x525/0x1120
  skb_mac_gso_segment+0x278/0x570

__udp_gso_segment_list() already has all the relevant data handy,
fix the issue traversing the segments list and updating the
GSO CB, if this is a tunnel GSO packet.

The issue is present since SKB_GSO_FRAGLIST introduction, but is
observable only since commit 18f25dc39990 ("udp: skip L4 aggregation
for UDP tunnel packets")

Fixes: 18f25dc39990 ("udp: skip L4 aggregation for UDP tunnel packets")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv4/udp_offload.c | 45 +++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 54e06b88af69..9f6022ba6bcd 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -214,8 +214,10 @@  static void __udpv4_gso_segment_csum(struct sk_buff *seg,
 	*oldip = *newip;
 }
 
-static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs)
+static void __udp_gso_segment_list_csum(struct sk_buff *segs, bool is_ipv6,
+					bool is_tunnel)
 {
+	bool update_csum = !is_ipv6;
 	struct sk_buff *seg;
 	struct udphdr *uh, *uh2;
 	struct iphdr *iph, *iph2;
@@ -224,31 +226,45 @@  static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs)
 	uh = udp_hdr(seg);
 	iph = ip_hdr(seg);
 
-	if ((udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) &&
+	if (update_csum && (udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) &&
 	    (udp_hdr(seg)->source == udp_hdr(seg->next)->source) &&
 	    (ip_hdr(seg)->daddr == ip_hdr(seg->next)->daddr) &&
 	    (ip_hdr(seg)->saddr == ip_hdr(seg->next)->saddr))
-		return segs;
+		update_csum = false;
+
+	if (!update_csum && !is_tunnel)
+		return;
 
+	/* this skb is CHECKSUM_NONE, if it has also a tunnel header
+	 * later __skb_udp_tunnel_segment() will try to make the
+	 * outer header csum and will need valid csum* fields
+	 */
+	SKB_GSO_CB(seg)->csum = ~uh->check;
+	SKB_GSO_CB(seg)->csum_start = seg->transport_header;
 	while ((seg = seg->next)) {
 		uh2 = udp_hdr(seg);
-		iph2 = ip_hdr(seg);
-
-		__udpv4_gso_segment_csum(seg,
-					 &iph2->saddr, &iph->saddr,
-					 &uh2->source, &uh->source);
-		__udpv4_gso_segment_csum(seg,
-					 &iph2->daddr, &iph->daddr,
-					 &uh2->dest, &uh->dest);
-	}
+		if (update_csum) {
+			iph2 = ip_hdr(seg);
+
+			__udpv4_gso_segment_csum(seg,
+						 &iph2->saddr, &iph->saddr,
+						 &uh2->source, &uh->source);
+			__udpv4_gso_segment_csum(seg,
+						 &iph2->daddr, &iph->daddr,
+						 &uh2->dest, &uh->dest);
+		}
 
-	return segs;
+		SKB_GSO_CB(seg)->csum = ~uh2->check;
+		SKB_GSO_CB(seg)->csum_start = seg->transport_header;
+	}
 }
 
 static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
 					      netdev_features_t features,
 					      bool is_ipv6)
 {
+	bool is_tunnel = !!(skb_shinfo(skb)->gso_type &
+			    (SKB_GSO_UDP_TUNNEL | SKB_GSO_UDP_TUNNEL_CSUM));
 	unsigned int mss = skb_shinfo(skb)->gso_size;
 
 	skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
@@ -257,7 +273,8 @@  static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
 
 	udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
 
-	return is_ipv6 ? skb : __udpv4_gso_segment_list_csum(skb);
+	__udp_gso_segment_list_csum(skb, is_ipv6, is_tunnel);
+	return skb;
 }
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,