* [PATCH net 1/3] xsk: validate checksum offload requests in copy mode
2026-10-21 18:03 [PATCH net 0/3] xsk, ipv6: validate user-supplied checksum offsets Nick Simpson
@ 2026-10-21 18:03 ` Nick Simpson
2026-10-21 18:03 ` [PATCH net 2/3] xsk: document checksum offload handling " Nick Simpson
2026-10-21 18:03 ` [PATCH net 3/3] ipv6: validate checksum_start before fragmenting Nick Simpson
2 siblings, 0 replies; 7+ messages in thread
From: Nick Simpson @ 2026-10-21 18:03 UTC (permalink / raw)
To: Magnus Karlsson, Maciej Fijalkowski, David Ahern, Ido Schimmel
Cc: Stanislav Fomichev, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Pablo Neira Ayuso,
Florian Westphal, Phil Sutter, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Michael S . Tsirkin, Willem de Bruijn, Jason Xing,
Song Yoong Siang, Rusty Russell, Max Krasnyansky,
Sridhar Samudrala, Jonathan Corbet, Shuah Khan, Randy Dunlap,
netdev, linux-doc, bpf, netfilter-devel, coreteam, linux-kernel
In copy mode, XDP_TXMD_FLAGS_CHECKSUM turns the csum_start and
csum_offset from the TX metadata into a CHECKSUM_PARTIAL skb.
xsk_skb_metadata() only checks that the checksum fits in the frame, so
csum_start can point into the Ethernet or IP header.
On a veth, the skb reaches the peer namespace still CHECKSUM_PARTIAL.
When that namespace forwards IPv6 with nf_defrag_ipv6 loaded,
nf_ct_frag6_reasm() moves the IPv6 header forward by the 8 bytes of the
fragment header without adjusting csum_start, the forward hook accepts
the reassembled packet, and ip6_fragment() then completes the checksum
with skb_checksum_help(), writing two bytes into the accepted header.
With csum_start = 22 and csum_offset = 22 on a fragmented IPv6/UDP
datagram from fd00:1::100, the write lands on source address bytes
14-15: an nftables forward rule matching "ip6 saddr fd00:1::100"
accepted the packet and the fragments sent out carried saddr
fd00:1::e87. For an unfragmented packet the same write happens at
egress when the output device does not offload the checksum; with
csum_start = 14 and csum_offset = 22 the packet left as fd00:1::ebf. A
csum_start that ends up before the network header (14 to 21 on the
fragmented path) makes skb_checksum_help() warn instead:
offset (-2) >= skb_headlen() (640)
WARNING: net/core/dev.c:3647 at skb_checksum_help+0x4c8/0x850
Call trace:
skb_checksum_help
ip6_fragment
ip6_finish_output
ip6_output
ip6_forward
ipv6_rcv
...
__dev_direct_xmit
__xsk_generic_xmit
xsk_sendmsg
socket(AF_XDP) only requires CAP_NET_RAW in the user namespace that owns
the network namespace, so the sender can be a container that owns one
end of the veth. With panic_on_warn, the warning panics the forwarding
host.
The interface is documented for L4 checksums. Leave CHECKSUM_PARTIAL in
place only when the request designates the TCP or UDP checksum field of
an unfragmented IPv4 or IPv6 packet, not IP-in-IP or GRE encapsulated,
on an Ethernet device, and complete any other request in software, as
XDP_UMEM_TX_SW_CSUM would, before the frame is handed to the device.
The frame is dissected from its Ethernet type, as skb->protocol is not
set for xsk skbs. The pending "net: resolve uncommon partial checksums
at ingress" applies the same rule to virtio_net_hdr sources.
A request on a frame that is itself an IP fragment is therefore
completed over that fragment alone, which is also all a device could
compute, so the reassembled datagram does not carry a valid transport
checksum. Before, if every fragment carried such a request, the
reassembled datagram stayed CHECKSUM_PARTIAL and was completed on
refragmentation; now conntrack marks it invalid.
Skbs without a linear part (IFF_TX_SKB_NO_LINEAR, i.e. virtio_net) are
left unchanged: their data stays in the umem, where user space can
still modify it after any check, and they go to the device rather than
to the local stack.
The problem was found with LLM-assisted code review.
Fixes: 48eb03dd2630 ("xsk: Add TX timestamp and TX checksum offload support")
Link: https://lore.kernel.org/r/9c08d12b3a7819ae03ded6a154e4f908276e93d4.1792411304.git.mst@redhat.com
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Nick Simpson <nick.simo@gmail.com>
---
net/xdp/xsk.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 781ba46eb152..a1cf701d42fc 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -11,6 +11,8 @@
#define pr_fmt(fmt) "AF_XDP: %s: " fmt, __func__
+#include <linux/if_arp.h>
+#include <linux/if_ether.h>
#include <linux/if_xdp.h>
#include <linux/init.h>
#include <linux/sched/mm.h>
@@ -21,6 +23,8 @@
#include <linux/uaccess.h>
#include <linux/net.h>
#include <linux/netdevice.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
#include <linux/rculist.h>
#include <linux/uio.h>
#include <linux/vmalloc.h>
@@ -28,6 +32,7 @@
#include <net/netdev_queues.h>
#include <net/xdp_sock_drv.h>
#include <net/busy_poll.h>
+#include <net/flow_dissector.h>
#include <net/netdev_lock.h>
#include <net/netdev_rx_queue.h>
#include <net/xdp.h>
@@ -985,6 +990,54 @@ static void xsk_drop_skb(struct sk_buff *skb)
xs->skb = NULL;
}
+/* Only TCP and UDP checksums of unfragmented IP packets that are not
+ * IP-in-IP or GRE encapsulated are left to the device. A CHECKSUM_PARTIAL
+ * skb can reach the local stack (e.g. through veth), and completing an
+ * arbitrary csum_start there may write into headers that were already
+ * inspected, e.g. after IPv6 reassembly. skb->protocol is not set for
+ * xsk skbs, so the frame is dissected from its Ethernet type instead.
+ */
+static bool xsk_csum_partial_ok(const struct sk_buff *skb)
+{
+ const struct ethhdr *eth = (const struct ethhdr *)skb->data;
+ struct flow_keys_basic keys;
+
+ if (skb->dev->type != ARPHRD_ETHER || skb_headlen(skb) < ETH_HLEN)
+ return false;
+
+ if (!skb_flow_dissect_flow_keys_basic(dev_net(skb->dev), NULL, &keys,
+ skb->data, eth->h_proto, ETH_HLEN,
+ skb_headlen(skb),
+ FLOW_DISSECTOR_F_STOP_AT_ENCAP))
+ return false;
+
+ if ((keys.control.flags & FLOW_DIS_IS_FRAGMENT) ||
+ skb_checksum_start_offset(skb) != keys.control.thoff)
+ return false;
+
+ switch (keys.basic.ip_proto) {
+ case IPPROTO_TCP:
+ return skb->csum_offset == offsetof(struct tcphdr, check);
+ case IPPROTO_UDP:
+ return skb->csum_offset == offsetof(struct udphdr, check);
+ default:
+ return false;
+ }
+}
+
+static int xsk_skb_resolve_csum(struct sk_buff *skb)
+{
+ /* Without a linear part (IFF_TX_SKB_NO_LINEAR), the frame stays in
+ * the umem, where user space can still change it, and goes to the
+ * device rather than the local stack: leave it as it is.
+ */
+ if (skb->ip_summed != CHECKSUM_PARTIAL || !skb_headlen(skb) ||
+ xsk_csum_partial_ok(skb))
+ return 0;
+
+ return skb_checksum_help(skb);
+}
+
static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
struct xdp_desc *desc, struct xsk_buff_pool *pool,
u32 hr)
@@ -1277,6 +1330,11 @@ static int __xsk_generic_xmit(struct sock *sk)
continue;
}
+ if (unlikely(xsk_skb_resolve_csum(skb))) {
+ xsk_drop_skb(skb);
+ continue;
+ }
+
err = __dev_direct_xmit(skb, xs->queue_id);
if (err == NETDEV_TX_BUSY) {
/* Tell user-space to retry the send */
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net 2/3] xsk: document checksum offload handling in copy mode
2026-10-21 18:03 [PATCH net 0/3] xsk, ipv6: validate user-supplied checksum offsets Nick Simpson
2026-10-21 18:03 ` [PATCH net 1/3] xsk: validate checksum offload requests in copy mode Nick Simpson
@ 2026-10-21 18:03 ` Nick Simpson
2026-10-21 18:03 ` [PATCH net 3/3] ipv6: validate checksum_start before fragmenting Nick Simpson
2 siblings, 0 replies; 7+ messages in thread
From: Nick Simpson @ 2026-10-21 18:03 UTC (permalink / raw)
To: Magnus Karlsson, Maciej Fijalkowski, David Ahern, Ido Schimmel
Cc: Stanislav Fomichev, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Pablo Neira Ayuso,
Florian Westphal, Phil Sutter, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Michael S . Tsirkin, Willem de Bruijn, Jason Xing,
Song Yoong Siang, Rusty Russell, Max Krasnyansky,
Sridhar Samudrala, Jonathan Corbet, Shuah Khan, Randy Dunlap,
netdev, linux-doc, bpf, netfilter-devel, coreteam, linux-kernel
In copy mode, XDP_TXMD_FLAGS_CHECKSUM requests are now left to the
device only for the TCP or UDP checksum of an unfragmented IPv4 or IPv6
packet, not IP-in-IP or GRE encapsulated, on an Ethernet device; other
requests are completed in software before transmission. On devices that
take skbs without a linear part (IFF_TX_SKB_NO_LINEAR), every request is
still left to the device. Say so in the description of the flag.
Assisted-by: LLM
Signed-off-by: Nick Simpson <nick.simo@gmail.com>
---
Documentation/networking/xsk-tx-metadata.rst | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/xsk-tx-metadata.rst b/Documentation/networking/xsk-tx-metadata.rst
index f240930ca1df..028564dba43d 100644
--- a/Documentation/networking/xsk-tx-metadata.rst
+++ b/Documentation/networking/xsk-tx-metadata.rst
@@ -50,7 +50,12 @@ The flags field enables the particular offload:
- ``XDP_TXMD_FLAGS_CHECKSUM``: requests the device to calculate L4
checksum. ``request.csum_start`` specifies byte offset of where the
checksumming should start and ``request.csum_offset`` specifies byte offset
- where the device should store the computed checksum.
+ where the device should store the computed checksum. In copy mode
+ (``XDP_COPY``), only the TCP or UDP checksum of an unfragmented IPv4 or
+ IPv6 packet, not IP-in-IP or GRE encapsulated, on an Ethernet device is
+ left to the device; other requests are completed in software before the
+ packet is transmitted. On devices that take skbs without a linear part
+ (``IFF_TX_SKB_NO_LINEAR``), every request is still left to the device.
- ``XDP_TXMD_FLAGS_LAUNCH_TIME``: requests the device to schedule the
packet for transmission at a pre-determined time called launch time. The
value of launch time is indicated by ``request.launch_time`` field of
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 3/3] ipv6: validate checksum_start before fragmenting
2026-10-21 18:03 [PATCH net 0/3] xsk, ipv6: validate user-supplied checksum offsets Nick Simpson
2026-10-21 18:03 ` [PATCH net 1/3] xsk: validate checksum offload requests in copy mode Nick Simpson
2026-10-21 18:03 ` [PATCH net 2/3] xsk: document checksum offload handling " Nick Simpson
@ 2026-10-21 18:03 ` Nick Simpson
2026-10-21 18:20 ` Eric Dumazet
2 siblings, 1 reply; 7+ messages in thread
From: Nick Simpson @ 2026-10-21 18:03 UTC (permalink / raw)
To: Magnus Karlsson, Maciej Fijalkowski, David Ahern, Ido Schimmel
Cc: Stanislav Fomichev, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Pablo Neira Ayuso,
Florian Westphal, Phil Sutter, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Michael S . Tsirkin, Willem de Bruijn, Jason Xing,
Song Yoong Siang, Rusty Russell, Max Krasnyansky,
Sridhar Samudrala, Jonathan Corbet, Shuah Khan, Randy Dunlap,
netdev, linux-doc, bpf, netfilter-devel, coreteam, linux-kernel
ip6_fragment() and br_ip6_fragment() complete CHECKSUM_PARTIAL skbs with
skb_checksum_help() without checking where csum_start points. After
reassembly by nf_defrag_ipv6, the IPv6 header has moved forward by the
8 bytes of the fragment header while csum_start has not, so a
CHECKSUM_PARTIAL skb from an untrusted source (tun/tap or packet sockets
with a virtio_net_hdr, or AF_XDP TX metadata) can have csum_start inside
the IPv6 header, and the checksum is then written into a header that the
forward hook has already accepted. When csum_start lies before the
network header, skb_checksum_help() warns.
Add the IPv6 counterpart of ip_check_csum_start() from
commit 6785011f8b16 ("ipv4: validate checksum_start before completing
checksum") to the two IPv6 fragmentation paths: refuse to complete a
checksum that starts inside the unfragmentable part of the packet (the
IPv6 header and the extension headers before the fragment header) and
drop the packet instead.
For IPv4, ip_check_csum_start() is also called from nfnetlink_queue,
xt_CHECKSUM and xfrm_output; their IPv6 counterparts are not changed
here. A reassembled packet that is not fragmented again, such as an
atomic fragment, and an unfragmented packet whose checksum is completed
at egress are not covered either; those have to be handled where the
CHECKSUM_PARTIAL skb is created, or by a check at reassembly time.
The problem was found with LLM-assisted code review.
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Fixes: 48eb03dd2630 ("xsk: Add TX timestamp and TX checksum offload support")
Assisted-by: LLM
Signed-off-by: Nick Simpson <nick.simo@gmail.com>
---
include/net/ipv6.h | 12 ++++++++++++
net/ipv6/ip6_output.c | 10 +++++++---
net/ipv6/netfilter.c | 10 +++++++---
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 3de07e738538..949695cb6764 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1029,6 +1029,18 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr);
+/* Refuse to complete a checksum that starts inside the unfragmentable
+ * part of the packet, i.e. the IPv6 header and the extension headers
+ * that precede the fragment header.
+ */
+static inline int ip6_check_csum_start(const struct sk_buff *skb,
+ unsigned int hlen)
+{
+ if (unlikely(skb->csum_start < skb->network_header + hlen))
+ return -EINVAL;
+ return 0;
+}
+
int ip6_append_data(struct sock *sk,
int getfrag(void *from, char *to, int offset, int len,
int odd, struct sk_buff *skb),
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 550965058991..55d7af9084d5 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -942,9 +942,13 @@ int ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
&ipv6_hdr(skb)->saddr);
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto fail;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ err = ip6_check_csum_start(skb, hlen);
+ if (!err)
+ err = skb_checksum_help(skb);
+ if (err)
+ goto fail;
+ }
prevhdr = skb_network_header(skb) + nexthdr_offset;
hroom = LL_RESERVED_SPACE(rt->dst.dev);
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index a7025ec87035..8476e4ba399c 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -144,9 +144,13 @@ int br_ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
&ipv6_hdr(skb)->saddr);
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto blackhole;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ err = ip6_check_csum_start(skb, hlen);
+ if (!err)
+ err = skb_checksum_help(skb);
+ if (err)
+ goto blackhole;
+ }
prevhdr = skb_network_header(skb) + nexthdr_offset;
hroom = LL_RESERVED_SPACE(skb->dev);
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net 3/3] ipv6: validate checksum_start before fragmenting
2026-10-21 18:03 ` [PATCH net 3/3] ipv6: validate checksum_start before fragmenting Nick Simpson
@ 2026-10-21 18:20 ` Eric Dumazet
2026-10-21 18:47 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2026-10-21 18:20 UTC (permalink / raw)
To: Nick Simpson
Cc: Magnus Karlsson, Maciej Fijalkowski, David Ahern, Ido Schimmel,
Stanislav Fomichev, David S . Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Michael S . Tsirkin,
Willem de Bruijn, Jason Xing, Song Yoong Siang, Rusty Russell,
Max Krasnyansky, Sridhar Samudrala, Jonathan Corbet, Shuah Khan,
Randy Dunlap, netdev, linux-doc, bpf, netfilter-devel, coreteam,
linux-kernel
Le mer. 21 oct. 2026 à 20:05, Nick Simpson <nick.simo@gmail.com> a écrit :
>
> ip6_fragment() and br_ip6_fragment() complete CHECKSUM_PARTIAL skbs with
> skb_checksum_help() without checking where csum_start points. After
> reassembly by nf_defrag_ipv6, the IPv6 header has moved forward by the
> 8 bytes of the fragment header while csum_start has not, so a
> CHECKSUM_PARTIAL skb from an untrusted source (tun/tap or packet sockets
> with a virtio_net_hdr, or AF_XDP TX metadata) can have csum_start inside
> the IPv6 header, and the checksum is then written into a header that the
> forward hook has already accepted. When csum_start lies before the
> network header, skb_checksum_help() warns.
>
> Add the IPv6 counterpart of ip_check_csum_start() from
> commit 6785011f8b16 ("ipv4: validate checksum_start before completing
> checksum") to the two IPv6 fragmentation paths: refuse to complete a
> checksum that starts inside the unfragmentable part of the packet (the
> IPv6 header and the extension headers before the fragment header) and
> drop the packet instead.
>
> For IPv4, ip_check_csum_start() is also called from nfnetlink_queue,
> xt_CHECKSUM and xfrm_output; their IPv6 counterparts are not changed
> here. A reassembled packet that is not fragmented again, such as an
> atomic fragment, and an unfragmented packet whose checksum is completed
> at egress are not covered either; those have to be handled where the
> CHECKSUM_PARTIAL skb is created, or by a check at reassembly time.
Then let's do that, instead of adding checks to every consumer.
A CHECKSUM_PARTIAL fragment is meaningless: the L4 checksum covers the
whole datagram, and our own stack completes it before fragmenting
(ip_do_fragment(), ip6_fragment()). Only untrusted injectors produce
them. Yet inet_frag_reasm_finish() keeps the reassembled skb
CHECKSUM_PARTIAL when all fragments are.
Downgrading CHECKSUM_PARTIAL to CHECKSUM_NONE in inet_frag_reasm_finish()
covers IPv4, IPv6, nf_defrag_ipv6 (and the bridge path) in one place;
the bogus L4 checksum is then verified and the packet dropped.
Also, reassembly does not move the L4 bytes, so a valid csum_start stays
valid; the "header moved without adjusting csum_start" framing is not
the bug.
Finally, this patch is described as defence in depth and not for
stable, but carries three Fixes: tags. Please drop it.
Nacked-by: Eric Dumazet <edumazet@kernel.org>
I am so tired of these LLM patches working around the issues instead
of fixing the root causes.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net 3/3] ipv6: validate checksum_start before fragmenting
2026-10-21 18:20 ` Eric Dumazet
@ 2026-10-21 18:47 ` Eric Dumazet
2026-10-21 20:02 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2026-10-21 18:47 UTC (permalink / raw)
To: Nick Simpson
Cc: Magnus Karlsson, Maciej Fijalkowski, David Ahern, Ido Schimmel,
Stanislav Fomichev, David S . Miller, Jakub Kicinski,
Paolo Abeni, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Michael S . Tsirkin,
Willem de Bruijn, Jason Xing, Song Yoong Siang, Rusty Russell,
Max Krasnyansky, Sridhar Samudrala, Jonathan Corbet, Shuah Khan,
Randy Dunlap, netdev, linux-doc, bpf, netfilter-devel, coreteam,
linux-kernel
Le mer. 21 oct. 2026 à 20:20, Eric Dumazet <edumazet@kernel.org> a écrit :
>
> Le mer. 21 oct. 2026 à 20:05, Nick Simpson <nick.simo@gmail.com> a écrit :
> >
> > ip6_fragment() and br_ip6_fragment() complete CHECKSUM_PARTIAL skbs with
> > skb_checksum_help() without checking where csum_start points. After
> > reassembly by nf_defrag_ipv6, the IPv6 header has moved forward by the
> > 8 bytes of the fragment header while csum_start has not, so a
> > CHECKSUM_PARTIAL skb from an untrusted source (tun/tap or packet sockets
> > with a virtio_net_hdr, or AF_XDP TX metadata) can have csum_start inside
> > the IPv6 header, and the checksum is then written into a header that the
> > forward hook has already accepted. When csum_start lies before the
> > network header, skb_checksum_help() warns.
> >
> > Add the IPv6 counterpart of ip_check_csum_start() from
> > commit 6785011f8b16 ("ipv4: validate checksum_start before completing
> > checksum") to the two IPv6 fragmentation paths: refuse to complete a
> > checksum that starts inside the unfragmentable part of the packet (the
> > IPv6 header and the extension headers before the fragment header) and
> > drop the packet instead.
> >
> > For IPv4, ip_check_csum_start() is also called from nfnetlink_queue,
> > xt_CHECKSUM and xfrm_output; their IPv6 counterparts are not changed
> > here. A reassembled packet that is not fragmented again, such as an
> > atomic fragment, and an unfragmented packet whose checksum is completed
> > at egress are not covered either; those have to be handled where the
> > CHECKSUM_PARTIAL skb is created, or by a check at reassembly time.
>
> Then let's do that, instead of adding checks to every consumer.
>
> A CHECKSUM_PARTIAL fragment is meaningless: the L4 checksum covers the
> whole datagram, and our own stack completes it before fragmenting
> (ip_do_fragment(), ip6_fragment()). Only untrusted injectors produce
> them. Yet inet_frag_reasm_finish() keeps the reassembled skb
> CHECKSUM_PARTIAL when all fragments are.
>
> Downgrading CHECKSUM_PARTIAL to CHECKSUM_NONE in inet_frag_reasm_finish()
> covers IPv4, IPv6, nf_defrag_ipv6 (and the bridge path) in one place;
> the bogus L4 checksum is then verified and the packet dropped.
>
>
Scratch that suggestion, it would be one more consumer-side check.
The fix belongs where CHECKSUM_PARTIAL skbs are created, and Michael
is already doing this for virtio_net_hdr sources (tun, tap, packet
sockets, virtio_net):
https://lore.kernel.org/netdev/cover.1792411304.git.mst@redhat.com/
In particular "net: resolve uncommon partial checksums at ingress":
https://lore.kernel.org/netdev/9c08d12b3a7819ae03ded6a154e4f908276e93d4.1792411304.git.mst@redhat.com/
CHECKSUM_PARTIAL is only kept when csum_start/csum_offset match a
parsed TCP or UDP header, and fragments are explicitly refused
(ip_is_fragment(), IP6_FH_F_FRAG): their checksum is completed at
ingress, before any protocol processing.
Your patch 1/3 applies the same rule to AF_XDP copy mode, and already
links to Michael's patch.
With both, no untrusted source can feed a CHECKSUM_PARTIAL fragment
to reassembly, nor an unfragmented packet with a bogus csum_start, so
neither this patch nor a reassembly change is needed.
Once the ingress validation is merged, the ip_check_csum_start() calls
added by commit 6785011f8b16 can be removed as well.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net 3/3] ipv6: validate checksum_start before fragmenting
2026-10-21 18:47 ` Eric Dumazet
@ 2026-10-21 20:02 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2026-10-21 20:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: Nick Simpson, Magnus Karlsson, Maciej Fijalkowski, David Ahern,
Ido Schimmel, Stanislav Fomichev, David S . Miller,
Jakub Kicinski, Paolo Abeni, Simon Horman, Pablo Neira Ayuso,
Florian Westphal, Phil Sutter, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Willem de Bruijn, Jason Xing, Song Yoong Siang, Rusty Russell,
Max Krasnyansky, Sridhar Samudrala, Jonathan Corbet, Shuah Khan,
Randy Dunlap, netdev, linux-doc, bpf, netfilter-devel, coreteam,
linux-kernel
On Wed, Oct 21, 2026 at 08:47:40PM +0200, Eric Dumazet wrote:
> Le mer. 21 oct. 2026 à 20:20, Eric Dumazet <edumazet@kernel.org> a écrit :
> >
> > Le mer. 21 oct. 2026 à 20:05, Nick Simpson <nick.simo@gmail.com> a écrit :
> > >
> > > ip6_fragment() and br_ip6_fragment() complete CHECKSUM_PARTIAL skbs with
> > > skb_checksum_help() without checking where csum_start points. After
> > > reassembly by nf_defrag_ipv6, the IPv6 header has moved forward by the
> > > 8 bytes of the fragment header while csum_start has not, so a
> > > CHECKSUM_PARTIAL skb from an untrusted source (tun/tap or packet sockets
> > > with a virtio_net_hdr, or AF_XDP TX metadata) can have csum_start inside
> > > the IPv6 header, and the checksum is then written into a header that the
> > > forward hook has already accepted. When csum_start lies before the
> > > network header, skb_checksum_help() warns.
> > >
> > > Add the IPv6 counterpart of ip_check_csum_start() from
> > > commit 6785011f8b16 ("ipv4: validate checksum_start before completing
> > > checksum") to the two IPv6 fragmentation paths: refuse to complete a
> > > checksum that starts inside the unfragmentable part of the packet (the
> > > IPv6 header and the extension headers before the fragment header) and
> > > drop the packet instead.
> > >
> > > For IPv4, ip_check_csum_start() is also called from nfnetlink_queue,
> > > xt_CHECKSUM and xfrm_output; their IPv6 counterparts are not changed
> > > here. A reassembled packet that is not fragmented again, such as an
> > > atomic fragment, and an unfragmented packet whose checksum is completed
> > > at egress are not covered either; those have to be handled where the
> > > CHECKSUM_PARTIAL skb is created, or by a check at reassembly time.
> >
> > Then let's do that, instead of adding checks to every consumer.
> >
> > A CHECKSUM_PARTIAL fragment is meaningless: the L4 checksum covers the
> > whole datagram, and our own stack completes it before fragmenting
> > (ip_do_fragment(), ip6_fragment()). Only untrusted injectors produce
> > them. Yet inet_frag_reasm_finish() keeps the reassembled skb
> > CHECKSUM_PARTIAL when all fragments are.
> >
> > Downgrading CHECKSUM_PARTIAL to CHECKSUM_NONE in inet_frag_reasm_finish()
> > covers IPv4, IPv6, nf_defrag_ipv6 (and the bridge path) in one place;
> > the bogus L4 checksum is then verified and the packet dropped.
> >
> >
>
> Scratch that suggestion, it would be one more consumer-side check.
>
> The fix belongs where CHECKSUM_PARTIAL skbs are created, and Michael
> is already doing this for virtio_net_hdr sources (tun, tap, packet
> sockets, virtio_net):
>
> https://lore.kernel.org/netdev/cover.1792411304.git.mst@redhat.com/
>
> In particular "net: resolve uncommon partial checksums at ingress":
> https://lore.kernel.org/netdev/9c08d12b3a7819ae03ded6a154e4f908276e93d4.1792411304.git.mst@redhat.com/
> CHECKSUM_PARTIAL is only kept when csum_start/csum_offset match a
> parsed TCP or UDP header, and fragments are explicitly refused
> (ip_is_fragment(), IP6_FH_F_FRAG): their checksum is completed at
> ingress, before any protocol processing.
>
> Your patch 1/3 applies the same rule to AF_XDP copy mode, and already
> links to Michael's patch.
Oh great, AF_XDP. Another one no one noticed.
> With both, no untrusted source can feed a CHECKSUM_PARTIAL fragment
> to reassembly, nor an unfragmented packet with a bogus csum_start, so
> neither this patch nor a reassembly change is needed.
>
> Once the ingress validation is merged, the ip_check_csum_start() calls
> added by commit 6785011f8b16 can be removed as well.
I think this would be a mistake. While plugging all places where some
packet change can confuse the stack is hard, making at least sure
there's no slab oob if we missed one is a no brainer imho: this is the
extent of the overhead:
if (unlikely(skb->csum_start < skb->network_header + ip_hdrlen(skb)))
return -EINVAL;
return 0;
I will be frank, messing up one of the subtle checks we are adding in
virtio_net_hdr_from_skb is *trivially easy*. It is getting them right
that is hard. Can we pretty please get help in not turning every such
mistake into a publicity stunt with most major cloud vendors affected by
guest driven host code execution?
I know, we should write less bugs (
Still, you guys are the net maintainers.
--
MST
^ permalink raw reply [flat|nested] 7+ messages in thread