| 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
| ||
|
Message-Id: <20240725-udp-gso-egress-from-tunnel-v1-1-5e5530ead524@cloudflare.com>
Date: Thu, 25 Jul 2024 11:55:54 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Willem de Bruijn <willemb@...gle.com>,
kernel-team@...udflare.com,
syzbot+e15b7e15b8a751a91d9a@...kaller.appspotmail.com
Subject: [PATCH net 1/2] udp: Mark GSO packets as CHECKSUM_UNNECESSARY
early on on output
In commit 10154dbded6d ("udp: Allow GSO transmit from devices with no
checksum offload") we have added a tweak in the UDP GSO code to mark GSO
packets being sent out as CHECKSUM_UNNECESSARY when the egress device
doesn't support checksum offload. This was done to satisfy the offload
checks in the gso stack.
However, when sending a UDP GSO packet from a tunnel device, we will go
through the TX path and the GSO offload twice. Once for the tunnel device,
which acts as a passthru for GSO packets, and once for the underlying
egress device.
Even though a tunnel device acts as a passthru for a UDP GSO packet, GSO
offload checks still happen on transmit from a tunnel device. So if the skb
is not marked as CHECKSUM_UNNECESSARY or CHECKSUM_PARTIAL, we will get a
warning from the gso stack.
Today this can occur in two situations, which we check for in
__ip_append_data() and __ip6_append_data():
1) when the tunnel device does not advertise checksum offload, or
2) when there are IPv6 extension headers present.
Syzbot has triggered the second case, producing a warning as below:
ip6tnl0: caps=(0x00000006401d7869, 0x00000006401d7869)
WARNING: CPU: 0 PID: 5112 at net/core/dev.c:3293 skb_warn_bad_offload+0x166/0x1a0 net/core/dev.c:3291
Modules linked in:
CPU: 0 PID: 5112 Comm: syz-executor391 Not tainted 6.10.0-rc7-syzkaller-01603-g80ab5445da62 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024
RIP: 0010:skb_warn_bad_offload+0x166/0x1a0 net/core/dev.c:3291
[...]
Call Trace:
<TASK>
__skb_gso_segment+0x3be/0x4c0 net/core/gso.c:127
skb_gso_segment include/net/gso.h:83 [inline]
validate_xmit_skb+0x585/0x1120 net/core/dev.c:3661
__dev_queue_xmit+0x17a4/0x3e90 net/core/dev.c:4415
neigh_output include/net/neighbour.h:542 [inline]
ip6_finish_output2+0xffa/0x1680 net/ipv6/ip6_output.c:137
ip6_finish_output+0x41e/0x810 net/ipv6/ip6_output.c:222
ip6_send_skb+0x112/0x230 net/ipv6/ip6_output.c:1958
udp_v6_send_skb+0xbf5/0x1870 net/ipv6/udp.c:1292
udpv6_sendmsg+0x23b3/0x3270 net/ipv6/udp.c:1588
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0xef/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2585
___sys_sendmsg net/socket.c:2639 [inline]
__sys_sendmmsg+0x3b2/0x740 net/socket.c:2725
__do_sys_sendmmsg net/socket.c:2754 [inline]
__se_sys_sendmmsg net/socket.c:2751 [inline]
__x64_sys_sendmmsg+0xa0/0xb0 net/socket.c:2751
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
[...]
</TASK>
To fix it mark UDP_GSO packets as CHECKSUM_UNNECESSARY early on the TX
path, when still in the udp layer, since we need to have ip_summed set up
correctly for GSO processing by tunnel devices.
Note that even if GSO packet gets marked as CHECKSUM_PARTIAL due to tunnel
advertising HW csum offload, it will not prevent software csum offload in
UDP GSO from kicking in if the underlying device doesn't offer csum
offload (for example, a TUN/TAP device with default config). This is
because we recheck device features in gso stack instead relying on the
ip_summed hint.
Fixes: 10154dbded6d ("udp: Allow GSO transmit from devices with no checksum offload")
Reported-by: syzbot+e15b7e15b8a751a91d9a@...kaller.appspotmail.com
Closes: https://lore.kernel.org/all/000000000000e1609a061d5330ce@google.com/
Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
---
net/ipv4/udp.c | 7 +++++++
net/ipv4/udp_offload.c | 8 --------
net/ipv6/udp.c | 7 +++++++
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 49c622e743e8..b7254b8a1e56 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -946,6 +946,13 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
}
if (datalen > cork->gso_size) {
+ /* On the TX path CHECKSUM_NONE and CHECKSUM_UNNECESSARY
+ * have the same meaning. However, check for bad
+ * offloads in the GSO stack expects the latter, if the
+ * checksum can be calculated in software.
+ */
+ if (skb->ip_summed == CHECKSUM_NONE)
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
skb_shinfo(skb)->gso_size = cork->gso_size;
skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index aa2e0a28ca61..59448a2dbf2c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -357,14 +357,6 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
else
uh->check = gso_make_checksum(seg, ~check) ? : CSUM_MANGLED_0;
- /* On the TX path, CHECKSUM_NONE and CHECKSUM_UNNECESSARY have the same
- * meaning. However, check for bad offloads in the GSO stack expects the
- * latter, if the checksum was calculated in software. To vouch for the
- * segment skbs we actually need to set it on the gso_skb.
- */
- if (gso_skb->ip_summed == CHECKSUM_NONE)
- gso_skb->ip_summed = CHECKSUM_UNNECESSARY;
-
/* update refcount for the packet */
if (copy_dtor) {
int delta = sum_truesize - gso_skb->truesize;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 6602a2e9cdb5..360392fc2b68 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1262,6 +1262,13 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
}
if (datalen > cork->gso_size) {
+ /* On the TX path CHECKSUM_NONE and CHECKSUM_UNNECESSARY
+ * have the same meaning. However, check for bad
+ * offloads in the GSO stack expects the latter, if the
+ * checksum can be calculated in software.
+ */
+ if (skb->ip_summed == CHECKSUM_NONE)
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
skb_shinfo(skb)->gso_size = cork->gso_size;
skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
--
2.40.1
Powered by blists - more mailing lists