Re: [PATCH net 1/3] xsk: validate checksum offload requests in copy mode

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Oct 22, 2026 at 01:07:59PM +0200, Björn Töpel wrote:
> Nick Simpson <nick.simo@gmail.com> writes:
> 
> > 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.
> 
> AI;DR. Ask you agent to be terse, please.

Or better read it please.

> Also, please do a proper write
> down of how to test/reproduce. The slop in the cover is no fun to read.
> :-(
> 
> > @@ -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;
> > +		}
> > +
> 
> Yes, we need to validate the csum offsets, similar to the virtio-net
> work MST is doing. Look what MST is doing in [1]. When that lands, move
> the helpers to ip.h as the TODO says, and use for XSK.

Yea now this is useful maybe I should add it directly there?
Or a patch on top is better after all?

The idea of a self-consistent small single patch died on the vine
at this point anyway...

Willem?


Anyway, pls post a patch on top of my series. I sent v5...

> 
> I *think* the IFF_TX_SKB_NO_LINEAR csum_partial scenario is OK, since
> we'll end up at a virtio device, but TBH I'm a bit skeptic about the
> whole reference to UMEM via an skb case...
> 
> 
> Björn
> 
> [1] 5de86f1afe1165a1a13ac4ef0615b19f4a10cb7a.1792565282.git.mst@redhat.com