* [PATCH net v4 0/2] net: prevent partial checksums from modifying IPv4 headers
@ 2026-09-20 0:47 Paulos Yibelo
2026-09-20 0:47 ` [PATCH net v4 1/2] net: validate virtio checksum start after network header Paulos Yibelo
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Paulos Yibelo @ 2026-09-20 0:47 UTC (permalink / raw)
To: netdev
Cc: mst, jasowangio, eperezma, xuanzhuo, virtualization, dsahern,
idosch, davem, edumazet, kuba, pabeni, horms, willemb, hannes,
linux-kernel
A TUN or virtio-net user can supply CHECKSUM_PARTIAL metadata whose
checksum start resolves inside the IPv4 header after link-layer headers
are removed. On the IPv4 fragmentation path, skb_checksum_help() may then
modify an IHL which was already parsed and validated. ip_do_fragment()
subsequently trusts the changed IHL and can copy beyond the skb's logical
linear head into emitted IPv4 options.
Patch 1 validates the checksum start relative to skb_network_header().
Patch 2 independently validates and retains the IPv4 header length before
checksum completion.
The issue and this series were reviewed privately. The source reproducer
and complete runtime evidence remain available privately.
Validation included strict checkpatch, focused W=1 builds, a complete
build, two test boots, a legitimate fragmented CHECKSUM_PARTIAL control,
and both forged cases.
Paulos Yibelo (2):
net: validate virtio checksum start after network header
ipv4: reject partial checksums covering the IP header
include/linux/virtio_net.h | 5 +++--
net/ipv4/ip_output.c | 23 +++++++++++++++++------
2 files changed, 20 insertions(+), 8 deletions(-)
base-commit: 9d565b6b72fe3f41fd43636e143072848105189f
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH net v4 1/2] net: validate virtio checksum start after network header 2026-09-20 0:47 [PATCH net v4 0/2] net: prevent partial checksums from modifying IPv4 headers Paulos Yibelo @ 2026-09-20 0:47 ` Paulos Yibelo 2026-09-20 1:11 ` David Ahern 2026-09-20 0:47 ` [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header Paulos Yibelo 2026-09-21 2:53 ` [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers Paulos Yibelo 2 siblings, 1 reply; 14+ messages in thread From: Paulos Yibelo @ 2026-09-20 0:47 UTC (permalink / raw) To: netdev Cc: mst, jasowangio, eperezma, xuanzhuo, virtualization, dsahern, idosch, davem, edumazet, kuba, pabeni, horms, willemb, hannes, linux-kernel __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than an estimated minimum network-header length. The comparison currently uses the offset from skb->data rather than the offset from skb_network_header(). For an AF_PACKET frame, skb->data can still point at the Ethernet header while skb_network_header() points past nested link-layer headers. A checksum start at the network header can therefore pass, then target byte zero after those headers are removed. This does not require a virtual-machine guest. A TUN device with virtio-net header support can supply the same checksum metadata. Keep the existing data-relative lower bound and also require checksum start to follow the estimated minimum relative to skb_network_header(). Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> --- Changes in v4: - State explicitly that a TUN device is sufficient and no guest is required, as noted by Michael S. Tsirkin. No code changes. Changes in v3: - Keep the network-relative comparison on one line for readability, as requested by David Ahern. Changes in v2: - Make nh_min_len an int and remove the casts, as suggested by Michael S. Tsirkin. include/linux/virtio_net.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index c381b91..a95ad46 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -52,7 +52,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, const struct virtio_net_hdr *hdr, bool little_endian, u8 hdr_gso_type) { - unsigned int nh_min_len = sizeof(struct iphdr); + int nh_min_len = sizeof(struct iphdr); unsigned int gso_type = 0; unsigned int thlen = 0; unsigned int p_off = 0; @@ -104,7 +104,8 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; - if (skb_transport_offset(skb) < nh_min_len) + if (skb_transport_offset(skb) < nh_min_len || + skb_transport_offset(skb) - skb_network_offset(skb) < nh_min_len) return -EINVAL; nh_min_len = skb_transport_offset(skb); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v4 1/2] net: validate virtio checksum start after network header 2026-09-20 0:47 ` [PATCH net v4 1/2] net: validate virtio checksum start after network header Paulos Yibelo @ 2026-09-20 1:11 ` David Ahern 0 siblings, 0 replies; 14+ messages in thread From: David Ahern @ 2026-09-20 1:11 UTC (permalink / raw) To: Paulos Yibelo, netdev Cc: mst, jasowangio, eperezma, xuanzhuo, virtualization, idosch, davem, edumazet, kuba, pabeni, horms, willemb, hannes, linux-kernel On 9/19/26 6:47 PM, Paulos Yibelo wrote: > __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than an > estimated minimum network-header length. The comparison currently uses the > offset from skb->data rather than the offset from skb_network_header(). > > For an AF_PACKET frame, skb->data can still point at the Ethernet header > while skb_network_header() points past nested link-layer headers. A > checksum start at the network header can therefore pass, then target byte > zero after those headers are removed. > > This does not require a virtual-machine guest. A TUN device with > virtio-net header support can supply the same checksum metadata. > > Keep the existing data-relative lower bound and also require checksum > start to follow the estimated minimum relative to skb_network_header(). > > Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > Cc: stable@vger.kernel.org > Assisted-by: LLM > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> > Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > Changes in v4: > - State explicitly that a TUN device is sufficient and no guest is required, > as noted by Michael S. Tsirkin. No code changes. > > Changes in v3: > - Keep the network-relative comparison on one line for readability, as > requested by David Ahern. > > Changes in v2: > - Make nh_min_len an int and remove the casts, as suggested by Michael S. > Tsirkin. > > include/linux/virtio_net.h | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header 2026-09-20 0:47 [PATCH net v4 0/2] net: prevent partial checksums from modifying IPv4 headers Paulos Yibelo 2026-09-20 0:47 ` [PATCH net v4 1/2] net: validate virtio checksum start after network header Paulos Yibelo @ 2026-09-20 0:47 ` Paulos Yibelo 2026-09-20 1:12 ` David Ahern 2026-09-21 2:53 ` [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers Paulos Yibelo 2 siblings, 1 reply; 14+ messages in thread From: Paulos Yibelo @ 2026-09-20 0:47 UTC (permalink / raw) To: netdev Cc: mst, jasowangio, eperezma, xuanzhuo, virtualization, dsahern, idosch, davem, edumazet, kuba, pabeni, horms, willemb, hannes, linux-kernel ip_do_fragment() completes a CHECKSUM_PARTIAL skb before reading the IPv4 header length. A virtualization interface can supply a checksum start that still points inside the IPv4 header after link-layer removal. This does not require a virtual-machine guest. A TUN device with virtio-net header support is sufficient to reach this path. skb_checksum_help() can then change iph->ihl after the packet was parsed and routed. Fragmentation trusts the changed IHL and can copy beyond the skb's logical linear head into transmitted IPv4 options. Read and validate IHL before checksum completion, reject a checksum start inside that header, retain the validated length, and reacquire iph after skb_checksum_help(). Fixes: dbd3393c56a8 ("ipv4: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> --- Changes in v4: - State explicitly that a TUN device is sufficient and no guest is required, as noted by Michael S. Tsirkin. No code changes. Changes in v3: - No code changes. Changes in v2: - No code changes. net/ipv4/ip_output.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index a24cc8e..ff902a2 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -770,17 +770,29 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, struct ip_frag_state state; int err = 0; - /* for offloaded checksums cleanup checksum before fragmentation */ - if (skb->ip_summed == CHECKSUM_PARTIAL && - (err = skb_checksum_help(skb))) - goto fail; - /* * Point into the IP datagram header. */ iph = ip_hdr(skb); + hlen = iph->ihl * 4; + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { + err = -EINVAL; + goto fail; + } + /* Complete offloaded checksums only after the validated IP header. */ + if (skb->ip_summed == CHECKSUM_PARTIAL) { + if (unlikely(skb_checksum_start_offset(skb) < hlen)) { + err = -EINVAL; + goto fail; + } + err = skb_checksum_help(skb); + if (err) + goto fail; + iph = ip_hdr(skb); + } + mtu = ip_skb_dst_mtu(sk, skb); if (IPCB(skb)->frag_max_size && IPCB(skb)->frag_max_size < mtu) mtu = IPCB(skb)->frag_max_size; @@ -789,7 +801,6 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, * Setup starting values. */ - hlen = iph->ihl * 4; if (mtu < hlen + 8) { err = -EMSGSIZE; goto fail; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header 2026-09-20 0:47 ` [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header Paulos Yibelo @ 2026-09-20 1:12 ` David Ahern 0 siblings, 0 replies; 14+ messages in thread From: David Ahern @ 2026-09-20 1:12 UTC (permalink / raw) To: Paulos Yibelo, netdev Cc: mst, jasowangio, eperezma, xuanzhuo, virtualization, idosch, davem, edumazet, kuba, pabeni, horms, willemb, hannes, linux-kernel On 9/19/26 6:47 PM, Paulos Yibelo wrote: > ip_do_fragment() completes a CHECKSUM_PARTIAL skb before reading the IPv4 > header length. A virtualization interface can supply a checksum start that > still points inside the IPv4 header after link-layer removal. > > This does not require a virtual-machine guest. A TUN device with > virtio-net header support is sufficient to reach this path. > > skb_checksum_help() can then change iph->ihl after the packet was parsed > and routed. Fragmentation trusts the changed IHL and can copy beyond the > skb's logical linear head into transmitted IPv4 options. > > Read and validate IHL before checksum completion, reject a checksum start > inside that header, retain the validated length, and reacquire iph after > skb_checksum_help(). > > Fixes: dbd3393c56a8 ("ipv4: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > Cc: stable@vger.kernel.org > Assisted-by: LLM > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> > Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > Changes in v4: > - State explicitly that a TUN device is sufficient and no guest is required, > as noted by Michael S. Tsirkin. No code changes. > > Changes in v3: > - No code changes. > > Changes in v2: > - No code changes. > > net/ipv4/ip_output.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > Reviewed-by: David Ahern <dsahern@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers 2026-09-20 0:47 [PATCH net v4 0/2] net: prevent partial checksums from modifying IPv4 headers Paulos Yibelo 2026-09-20 0:47 ` [PATCH net v4 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-20 0:47 ` [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header Paulos Yibelo @ 2026-09-21 2:53 ` Paulos Yibelo 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-21 2:53 ` [PATCH net v5 2/2] ip: reject partial checksums covering network headers Paulos Yibelo 2 siblings, 2 replies; 14+ messages in thread From: Paulos Yibelo @ 2026-09-21 2:53 UTC (permalink / raw) To: netdev Cc: richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, mst, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel A virtio-net header can supply CHECKSUM_PARTIAL metadata whose checksum start resolves inside the network header after link-layer removal. Software checksum completion can then modify header bytes which the stack has already parsed. Patch 1 validates the checksum start against an explicit data-relative L3 origin. It covers TUN/TAP, virtio-net, AF_PACKET, UML, nested VLAN headers, and tunnel metadata. It does not rely on skb header state which may not yet be established. Patch 2 independently validates the checksum start against the parsed IPv4 or IPv6 header length in all four IP fragmentation implementations which complete partial checksums. The v4 Sashiko findings were correct. Patch 1 used skb_network_offset() before all receive callers had established it. Patch 2 compared a signed checksum offset with an unsigned IPv4 header length. This revision fixes both findings and covers the corresponding bridge and IPv6 fragmentation paths. Validation included strict checkpatch, focused x86 and UML W=1 builds, an offset-boundary model, and application of the exact mail series to the stated base. Changes in v5: - Pass an explicit data-relative L3 origin through the virtio-net converter and audit every in-tree caller. - Parse Ethernet and nested VLAN headers without mutating skb header state. - Propagate virtio-header conversion failures in UML. - Keep the IPv4 comparison signed and add matching parsed-header checks to the IPv4/IPv6 output and bridge-netfilter fragmentation paths. - Drop Michael S. Tsirkin's Acked-by and David Ahern's Reviewed-by tags because both patches changed materially. Link: https://lore.kernel.org/netdev/20260920004733.6473-1-habte.yibelo@gmail.com/ Paulos Yibelo (2): net: validate virtio checksum start after network header ip: reject partial checksums covering network headers arch/um/drivers/vector_transports.c | 10 ++- drivers/net/tun_vnet.h | 28 +++++++- drivers/net/virtio_net.c | 8 ++- include/linux/virtio_net.h | 76 ++++++++++++++++++---- net/bridge/netfilter/nf_conntrack_bridge.c | 21 ++++-- net/ipv4/ip_output.c | 23 +++++-- net/ipv6/ip6_output.c | 12 +++- net/ipv6/netfilter.c | 12 +++- net/packet/af_packet.c | 6 +- 9 files changed, 157 insertions(+), 39 deletions(-) base-commit: 1e24c4f2ee44be0eee94092b5d13cbdb4bdf0d60 -- 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 2:53 ` [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers Paulos Yibelo @ 2026-09-21 2:53 ` Paulos Yibelo 2026-09-21 22:11 ` Michael S. Tsirkin ` (3 more replies) 2026-09-21 2:53 ` [PATCH net v5 2/2] ip: reject partial checksums covering network headers Paulos Yibelo 1 sibling, 4 replies; 14+ messages in thread From: Paulos Yibelo @ 2026-09-21 2:53 UTC (permalink / raw) To: netdev Cc: richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, mst, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than an estimated minimum network-header length. Its input offsets are relative to skb->data. Using skb_network_offset() here is unsafe. TUN/TAP, virtio-net, and UML parse a received virtio header before skb->network_header is established. On an skb with headroom, the resulting negative offset enlarges the apparent distance to the transport header and can admit a checksum start inside the network header. Pass the data-relative L3 offset to the converter explicitly. IFF_TUN uses zero, AF_PACKET supplies its established network offset, and Ethernet receive paths parse Ethernet and nested VLAN headers with skb_header_pointer(), without changing skb state. Use the same origin for tunnel-offset validation, and make UML propagate conversion failures. This does not require a virtual-machine guest. A TUN or TAP device with virtio-net header support is sufficient to reach these paths. Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ Cc: stable@vger.kernel.org Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> --- Changes in v5: - Replace the not-yet-established skb network-header offset with an explicit data-relative L3 origin. - Cover all in-tree callers, including Ethernet/VLAN receive paths, tunnel metadata, and UML error propagation. - Drop the prior Acked-by and Reviewed-by tags because the code changed. Changes in v4: - State that a TUN device is sufficient and no guest is required, as noted by Michael S. Tsirkin. Changes in v3: - Keep the network-relative comparison on one line for readability, as requested by David Ahern. Changes in v2: - Make nh_min_len an int and remove the casts, as suggested by Michael S. Tsirkin. arch/um/drivers/vector_transports.c | 10 +++- drivers/net/tun_vnet.h | 28 ++++++++++- drivers/net/virtio_net.c | 8 ++- include/linux/virtio_net.h | 76 +++++++++++++++++++++++------ net/packet/af_packet.c | 6 ++- 5 files changed, 106 insertions(+), 22 deletions(-) diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c index ddd127ee9..79bc05fc6 100644 --- a/arch/um/drivers/vector_transports.c +++ b/arch/um/drivers/vector_transports.c @@ -197,6 +197,7 @@ static int raw_verify_header( uint8_t *header, struct sk_buff *skb, struct vector_private *vp) { struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; + int network_offset; if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && (vp->req_size != 65536)) { @@ -209,8 +210,13 @@ static int raw_verify_header( if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) return 1; - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); - return 0; + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); + if (network_offset < 0) + return network_offset; + + return virtio_net_hdr_to_skb(skb, vheader, + virtio_legacy_is_little_endian(), + network_offset); } static bool get_uint_param( diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h index f4c652b1f..1c83c359d 100644 --- a/drivers/net/tun_vnet.h +++ b/drivers/net/tun_vnet.h @@ -177,10 +177,27 @@ static inline int tun_vnet_hdr_put(int sz, struct iov_iter *iter, return __tun_vnet_hdr_put(sz, 0, iter, hdr); } +static inline int +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff *skb, + const struct virtio_net_hdr *hdr) +{ + if ((flags & TUN_TYPE_MASK) != IFF_TAP) + return 0; + + return virtio_net_hdr_get_l3_offset(skb, hdr); +} + static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb, const struct virtio_net_hdr *hdr) { - return virtio_net_hdr_to_skb(skb, hdr, tun_vnet_is_little_endian(flags)); + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr); + + if (network_offset < 0) + return network_offset; + + return virtio_net_hdr_to_skb(skb, hdr, + tun_vnet_is_little_endian(flags), + network_offset); } /* @@ -199,10 +216,17 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features, struct sk_buff *skb, const struct virtio_net_hdr_v1_hash_tunnel *hdr) { + const struct virtio_net_hdr *vnet_hdr = (const struct virtio_net_hdr *)hdr; + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, vnet_hdr); + + if (network_offset < 0) + return network_offset; + return virtio_net_hdr_tnl_to_skb(skb, hdr, features & NETIF_F_GSO_UDP_TUNNEL, features & NETIF_F_GSO_UDP_TUNNEL_CSUM, - tun_vnet_is_little_endian(flags)); + tun_vnet_is_little_endian(flags), + network_offset); } static inline int tun_vnet_hdr_from_skb(unsigned int flags, diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e34c52d05..059eeb18e 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2502,6 +2502,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * { struct virtio_net_common_hdr *hdr; struct net_device *dev = vi->dev; + int network_offset; hdr = skb_vnet_common_hdr(skb); if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) @@ -2515,9 +2516,12 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * goto frame_err; } - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, + network_offset = virtio_net_hdr_get_l3_offset(skb, &hdr->hdr); + if (network_offset < 0 || + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, vi->rx_tnl_csum, - virtio_is_little_endian(vi->vdev))) { + virtio_is_little_endian(vi->vdev), + network_offset)) { net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", dev->name, hdr->hdr.gso_type, hdr->hdr.gso_size, hdr->hdr.flags, diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index c381b916c..a4c005796 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -48,11 +48,49 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, return 0; } +/* + * Return the L3 offset of an Ethernet frame starting at skb->data. + * The offset is unused without NEEDS_CSUM, so avoid parsing and return zero. + */ +static inline int +virtio_net_hdr_get_l3_offset(const struct sk_buff *skb, + const struct virtio_net_hdr *hdr) +{ + unsigned int parse_depth = VLAN_MAX_DEPTH; + const struct ethhdr *eth; + struct ethhdr ethbuf; + __be16 protocol; + int depth = ETH_HLEN; + + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) + return 0; + + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); + if (!eth) + return -EINVAL; + + protocol = eth->h_proto; + while (eth_type_vlan(protocol)) { + const struct vlan_hdr *vh; + struct vlan_hdr vhdr; + + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); + if (!vh || !--parse_depth) + return -EINVAL; + + protocol = vh->h_vlan_encapsulated_proto; + depth += VLAN_HLEN; + } + + return depth; +} + static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, const struct virtio_net_hdr *hdr, - bool little_endian, u8 hdr_gso_type) + bool little_endian, u8 hdr_gso_type, + int network_offset) { - unsigned int nh_min_len = sizeof(struct iphdr); + int nh_min_len = sizeof(struct iphdr); unsigned int gso_type = 0; unsigned int thlen = 0; unsigned int p_off = 0; @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); + int transport_offset; if (!pskb_may_pull(skb, needed)) return -EINVAL; if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; - if (skb_transport_offset(skb) < nh_min_len) + + transport_offset = skb_transport_offset(skb); + if (transport_offset < nh_min_len || network_offset < 0 || + network_offset > transport_offset - nh_min_len) return -EINVAL; - nh_min_len = skb_transport_offset(skb); + nh_min_len = transport_offset; p_off = nh_min_len + thlen; if (!pskb_may_pull(skb, p_off)) return -EINVAL; @@ -206,9 +248,11 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, const struct virtio_net_hdr *hdr, - bool little_endian) + bool little_endian, + int network_offset) { - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type); + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type, + network_offset); } /* This function must be called after virtio_net_hdr_from_skb(). */ @@ -287,7 +331,7 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, return 0; } -static inline unsigned int virtio_l3min(bool is_ipv6) +static inline int virtio_l3min(bool is_ipv6) { return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); } @@ -297,18 +341,19 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, const struct virtio_net_hdr_v1_hash_tunnel *vhdr, bool tnl_hdr_negotiated, bool tnl_csum_negotiated, - bool little_endian) + bool little_endian, int network_offset) { const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr; - unsigned int inner_nh, outer_th, inner_th; - unsigned int inner_l3min, outer_l3min; u8 gso_inner_type, gso_tunnel_type; bool outer_isv6, inner_isv6; + int inner_nh, outer_th, inner_th; + int inner_l3min, outer_l3min; int ret; gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; if (!gso_tunnel_type) - return virtio_net_hdr_to_skb(skb, hdr, little_endian); + return virtio_net_hdr_to_skb(skb, hdr, little_endian, + network_offset); /* Tunnel not supported/negotiated, but the hdr asks for it. */ if (!tnl_hdr_negotiated) @@ -332,19 +377,22 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; inner_l3min = virtio_l3min(inner_isv6); - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); + outer_l3min = virtio_l3min(outer_isv6); inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); inner_nh = le16_to_cpu(vhdr->inner_nh_offset); outer_th = le16_to_cpu(vhdr->outer_th_offset); - if (outer_th < outer_l3min || + if (network_offset < 0 || + outer_th < outer_l3min || + network_offset > outer_th - outer_l3min || inner_nh < outer_th + sizeof(struct udphdr) || inner_th < inner_nh + inner_l3min) return -EINVAL; /* Let the basic parsing deal with plain GSO features. */ ret = __virtio_net_hdr_to_skb(skb, hdr, true, - hdr->gso_type & ~gso_tunnel_type); + hdr->gso_type & ~gso_tunnel_type, + network_offset); if (ret) return ret; diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 50cae32ae..04c80e23d 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) } if (has_vnet_hdr) { - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), + skb_network_offset(skb))) { tp_len = -EINVAL; goto tpacket_error; } @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) packet_parse_headers(skb, sock); if (vnet_hdr_sz) { - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), + skb_network_offset(skb)); if (err) goto out_free; len += vnet_hdr_sz; -- 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo @ 2026-09-21 22:11 ` Michael S. Tsirkin 2026-09-21 22:18 ` Michael S. Tsirkin ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2026-09-21 22:11 UTC (permalink / raw) To: Paulos Yibelo Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel On Sun, Sep 20, 2026 at 10:53:40PM -0400, Paulos Yibelo wrote: > __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than > an estimated minimum network-header length. Its input offsets are relative > to skb->data. > > Using skb_network_offset() here is unsafe. TUN/TAP, virtio-net, and UML > parse a received virtio header before skb->network_header is established. > On an skb with headroom, the resulting negative offset enlarges the > apparent distance to the transport header and can admit a checksum start > inside the network header. > > Pass the data-relative L3 offset to the converter explicitly. IFF_TUN uses > zero, AF_PACKET supplies its established network offset, and Ethernet > receive paths parse Ethernet and nested VLAN headers with > skb_header_pointer(), without changing skb state. Use the same origin for > tunnel-offset validation, and make UML propagate conversion failures. > > This does not require a virtual-machine guest. A TUN or TAP device with > virtio-net header support is sufficient to reach these paths. > > Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.") > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ > Cc: stable@vger.kernel.org > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> v4 had assisted-by tag? same q for patch 2. > --- > Changes in v5: > - Replace the not-yet-established skb network-header offset with an > explicit data-relative L3 origin. > - Cover all in-tree callers, including Ethernet/VLAN receive paths, > tunnel metadata, and UML error propagation. > - Drop the prior Acked-by and Reviewed-by tags because the code changed. > > Changes in v4: > - State that a TUN device is sufficient and no guest is required, as > noted by Michael S. Tsirkin. > > Changes in v3: > - Keep the network-relative comparison on one line for readability, as > requested by David Ahern. > > Changes in v2: > - Make nh_min_len an int and remove the casts, as suggested by Michael S. > Tsirkin. > > arch/um/drivers/vector_transports.c | 10 +++- > drivers/net/tun_vnet.h | 28 ++++++++++- > drivers/net/virtio_net.c | 8 ++- > include/linux/virtio_net.h | 76 +++++++++++++++++++++++------ > net/packet/af_packet.c | 6 ++- > 5 files changed, 106 insertions(+), 22 deletions(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index ddd127ee9..79bc05fc6 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -197,6 +197,7 @@ static int raw_verify_header( > uint8_t *header, struct sk_buff *skb, struct vector_private *vp) > { > struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; > + int network_offset; > > if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && > (vp->req_size != 65536)) { > @@ -209,8 +210,13 @@ static int raw_verify_header( > if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) > return 1; > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > - return 0; > + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian(), > + network_offset); > } > > static bool get_uint_param( > diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h > index f4c652b1f..1c83c359d 100644 > --- a/drivers/net/tun_vnet.h > +++ b/drivers/net/tun_vnet.h > @@ -177,10 +177,27 @@ static inline int tun_vnet_hdr_put(int sz, struct iov_iter *iter, > return __tun_vnet_hdr_put(sz, 0, iter, hdr); > } > > +static inline int > +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) > +{ > + if ((flags & TUN_TYPE_MASK) != IFF_TAP) > + return 0; > + > + return virtio_net_hdr_get_l3_offset(skb, hdr); > +} > + > static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb, > const struct virtio_net_hdr *hdr) > { > - return virtio_net_hdr_to_skb(skb, hdr, tun_vnet_is_little_endian(flags)); > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr); > + > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, hdr, > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > /* > @@ -199,10 +216,17 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features, > struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *hdr) > { > + const struct virtio_net_hdr *vnet_hdr = (const struct virtio_net_hdr *)hdr; > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, vnet_hdr); > + > + if (network_offset < 0) > + return network_offset; > + > return virtio_net_hdr_tnl_to_skb(skb, hdr, > features & NETIF_F_GSO_UDP_TUNNEL, > features & NETIF_F_GSO_UDP_TUNNEL_CSUM, > - tun_vnet_is_little_endian(flags)); > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > static inline int tun_vnet_hdr_from_skb(unsigned int flags, > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index e34c52d05..059eeb18e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2502,6 +2502,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > { > struct virtio_net_common_hdr *hdr; > struct net_device *dev = vi->dev; > + int network_offset; > > hdr = skb_vnet_common_hdr(skb); > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > @@ -2515,9 +2516,12 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > goto frame_err; > } > > - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > + network_offset = virtio_net_hdr_get_l3_offset(skb, &hdr->hdr); > + if (network_offset < 0 || > + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > vi->rx_tnl_csum, > - virtio_is_little_endian(vi->vdev))) { > + virtio_is_little_endian(vi->vdev), > + network_offset)) { > net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", > dev->name, hdr->hdr.gso_type, > hdr->hdr.gso_size, hdr->hdr.flags, > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index c381b916c..a4c005796 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -48,11 +48,49 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, > return 0; > } > > +/* > + * Return the L3 offset of an Ethernet frame starting at skb->data. > + * The offset is unused without NEEDS_CSUM, so avoid parsing and return zero. > + */ > +static inline int > +virtio_net_hdr_get_l3_offset(const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) > +{ > + unsigned int parse_depth = VLAN_MAX_DEPTH; > + const struct ethhdr *eth; > + struct ethhdr ethbuf; > + __be16 protocol; > + int depth = ETH_HLEN; > + > + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) > + return 0; > + > + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); > + if (!eth) > + return -EINVAL; > + > + protocol = eth->h_proto; > + while (eth_type_vlan(protocol)) { > + const struct vlan_hdr *vh; > + struct vlan_hdr vhdr; > + > + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); > + if (!vh || !--parse_depth) > + return -EINVAL; > + > + protocol = vh->h_vlan_encapsulated_proto; > + depth += VLAN_HLEN; > + } > + > + return depth; > +} > + > static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian, u8 hdr_gso_type) > + bool little_endian, u8 hdr_gso_type, > + int network_offset) > { > - unsigned int nh_min_len = sizeof(struct iphdr); > + int nh_min_len = sizeof(struct iphdr); > unsigned int gso_type = 0; > unsigned int thlen = 0; > unsigned int p_off = 0; > @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > + int transport_offset; > > if (!pskb_may_pull(skb, needed)) > return -EINVAL; > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > - if (skb_transport_offset(skb) < nh_min_len) > + > + transport_offset = skb_transport_offset(skb); > + if (transport_offset < nh_min_len || network_offset < 0 || > + network_offset > transport_offset - nh_min_len) > return -EINVAL; > > - nh_min_len = skb_transport_offset(skb); > + nh_min_len = transport_offset; > p_off = nh_min_len + thlen; > if (!pskb_may_pull(skb, p_off)) > return -EINVAL; > @@ -206,9 +248,11 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > > static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian) > + bool little_endian, > + int network_offset) > { > - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type); > + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type, > + network_offset); > } > > /* This function must be called after virtio_net_hdr_from_skb(). */ > @@ -287,7 +331,7 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > return 0; > } > > -static inline unsigned int virtio_l3min(bool is_ipv6) > +static inline int virtio_l3min(bool is_ipv6) > { > return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); > } > @@ -297,18 +341,19 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *vhdr, > bool tnl_hdr_negotiated, > bool tnl_csum_negotiated, > - bool little_endian) > + bool little_endian, int network_offset) > { > const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr; > - unsigned int inner_nh, outer_th, inner_th; > - unsigned int inner_l3min, outer_l3min; > u8 gso_inner_type, gso_tunnel_type; > bool outer_isv6, inner_isv6; > + int inner_nh, outer_th, inner_th; > + int inner_l3min, outer_l3min; > int ret; > > gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; > if (!gso_tunnel_type) > - return virtio_net_hdr_to_skb(skb, hdr, little_endian); > + return virtio_net_hdr_to_skb(skb, hdr, little_endian, > + network_offset); > > /* Tunnel not supported/negotiated, but the hdr asks for it. */ > if (!tnl_hdr_negotiated) > @@ -332,19 +377,22 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; > inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; > inner_l3min = virtio_l3min(inner_isv6); > - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); > + outer_l3min = virtio_l3min(outer_isv6); > > inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); > inner_nh = le16_to_cpu(vhdr->inner_nh_offset); > outer_th = le16_to_cpu(vhdr->outer_th_offset); > - if (outer_th < outer_l3min || > + if (network_offset < 0 || > + outer_th < outer_l3min || > + network_offset > outer_th - outer_l3min || > inner_nh < outer_th + sizeof(struct udphdr) || > inner_th < inner_nh + inner_l3min) > return -EINVAL; > > /* Let the basic parsing deal with plain GSO features. */ > ret = __virtio_net_hdr_to_skb(skb, hdr, true, > - hdr->gso_type & ~gso_tunnel_type); > + hdr->gso_type & ~gso_tunnel_type, > + network_offset); > if (ret) > return ret; > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 50cae32ae..04c80e23d 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) > } > > if (has_vnet_hdr) { > - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { > + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb))) { > tp_len = -EINVAL; > goto tpacket_error; > } > @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) > packet_parse_headers(skb, sock); > > if (vnet_hdr_sz) { > - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); > + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb)); > if (err) > goto out_free; > len += vnet_hdr_sz; > -- > 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-21 22:11 ` Michael S. Tsirkin @ 2026-09-21 22:18 ` Michael S. Tsirkin 2026-09-21 22:44 ` Michael S. Tsirkin 2026-09-24 8:54 ` netdev-bot+sashiko 3 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2026-09-21 22:18 UTC (permalink / raw) To: Paulos Yibelo Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel On Sun, Sep 20, 2026 at 10:53:40PM -0400, Paulos Yibelo wrote: > __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than > an estimated minimum network-header length. Its input offsets are relative > to skb->data. > > Using skb_network_offset() here is unsafe. TUN/TAP, virtio-net, and UML > parse a received virtio header before skb->network_header is established. > On an skb with headroom, the resulting negative offset enlarges the > apparent distance to the transport header and can admit a checksum start > inside the network header. > > Pass the data-relative L3 offset to the converter explicitly. IFF_TUN uses > zero, AF_PACKET supplies its established network offset, and Ethernet > receive paths parse Ethernet and nested VLAN headers with > skb_header_pointer(), without changing skb state. Use the same origin for > tunnel-offset validation, and make UML propagate conversion failures. > > This does not require a virtual-machine guest. A TUN or TAP device with > virtio-net header support is sufficient to reach these paths. > > Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.") > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ > Cc: stable@vger.kernel.org > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> > --- > Changes in v5: > - Replace the not-yet-established skb network-header offset with an > explicit data-relative L3 origin. > - Cover all in-tree callers, including Ethernet/VLAN receive paths, > tunnel metadata, and UML error propagation. > - Drop the prior Acked-by and Reviewed-by tags because the code changed. > > Changes in v4: > - State that a TUN device is sufficient and no guest is required, as > noted by Michael S. Tsirkin. > > Changes in v3: > - Keep the network-relative comparison on one line for readability, as > requested by David Ahern. > > Changes in v2: > - Make nh_min_len an int and remove the casts, as suggested by Michael S. > Tsirkin. > > arch/um/drivers/vector_transports.c | 10 +++- > drivers/net/tun_vnet.h | 28 ++++++++++- > drivers/net/virtio_net.c | 8 ++- > include/linux/virtio_net.h | 76 +++++++++++++++++++++++------ > net/packet/af_packet.c | 6 ++- > 5 files changed, 106 insertions(+), 22 deletions(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index ddd127ee9..79bc05fc6 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -197,6 +197,7 @@ static int raw_verify_header( > uint8_t *header, struct sk_buff *skb, struct vector_private *vp) > { > struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; > + int network_offset; > > if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && > (vp->req_size != 65536)) { > @@ -209,8 +210,13 @@ static int raw_verify_header( > if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) > return 1; > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > - return 0; > + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian(), > + network_offset); > } > > static bool get_uint_param( > diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h > index f4c652b1f..1c83c359d 100644 > --- a/drivers/net/tun_vnet.h > +++ b/drivers/net/tun_vnet.h > @@ -177,10 +177,27 @@ static inline int tun_vnet_hdr_put(int sz, struct iov_iter *iter, > return __tun_vnet_hdr_put(sz, 0, iter, hdr); > } > > +static inline int > +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) > +{ > + if ((flags & TUN_TYPE_MASK) != IFF_TAP) > + return 0; > + > + return virtio_net_hdr_get_l3_offset(skb, hdr); > +} > + > static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb, > const struct virtio_net_hdr *hdr) > { > - return virtio_net_hdr_to_skb(skb, hdr, tun_vnet_is_little_endian(flags)); > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr); > + > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, hdr, > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > /* > @@ -199,10 +216,17 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features, > struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *hdr) > { > + const struct virtio_net_hdr *vnet_hdr = (const struct virtio_net_hdr *)hdr; > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, vnet_hdr); > + > + if (network_offset < 0) > + return network_offset; > + > return virtio_net_hdr_tnl_to_skb(skb, hdr, > features & NETIF_F_GSO_UDP_TUNNEL, > features & NETIF_F_GSO_UDP_TUNNEL_CSUM, > - tun_vnet_is_little_endian(flags)); > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > static inline int tun_vnet_hdr_from_skb(unsigned int flags, > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index e34c52d05..059eeb18e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2502,6 +2502,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > { > struct virtio_net_common_hdr *hdr; > struct net_device *dev = vi->dev; > + int network_offset; > > hdr = skb_vnet_common_hdr(skb); > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > @@ -2515,9 +2516,12 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > goto frame_err; > } > > - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > + network_offset = virtio_net_hdr_get_l3_offset(skb, &hdr->hdr); > + if (network_offset < 0 || > + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > vi->rx_tnl_csum, > - virtio_is_little_endian(vi->vdev))) { > + virtio_is_little_endian(vi->vdev), > + network_offset)) { > net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", > dev->name, hdr->hdr.gso_type, > hdr->hdr.gso_size, hdr->hdr.flags, > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index c381b916c..a4c005796 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -48,11 +48,49 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, > return 0; > } > > +/* > + * Return the L3 offset of an Ethernet frame starting at skb->data. > + * The offset is unused without NEEDS_CSUM, so avoid parsing and return zero. > + */ > +static inline int > +virtio_net_hdr_get_l3_offset(const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) +virtio_net_hdr_eth_get_l3_offset ? since this assumes ethernet... > +{ > + unsigned int parse_depth = VLAN_MAX_DEPTH; > + const struct ethhdr *eth; > + struct ethhdr ethbuf; > + __be16 protocol; > + int depth = ETH_HLEN; > + > + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) > + return 0; > + > + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); > + if (!eth) > + return -EINVAL; > + > + protocol = eth->h_proto; > + while (eth_type_vlan(protocol)) { > + const struct vlan_hdr *vh; > + struct vlan_hdr vhdr; > + > + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); > + if (!vh || !--parse_depth) > + return -EINVAL; > + > + protocol = vh->h_vlan_encapsulated_proto; > + depth += VLAN_HLEN; > + } > + > + return depth; > +} > + > static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian, u8 hdr_gso_type) > + bool little_endian, u8 hdr_gso_type, > + int network_offset) > { > - unsigned int nh_min_len = sizeof(struct iphdr); > + int nh_min_len = sizeof(struct iphdr); > unsigned int gso_type = 0; > unsigned int thlen = 0; > unsigned int p_off = 0; > @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > + int transport_offset; > > if (!pskb_may_pull(skb, needed)) > return -EINVAL; > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > - if (skb_transport_offset(skb) < nh_min_len) > + > + transport_offset = skb_transport_offset(skb); > + if (transport_offset < nh_min_len || network_offset < 0 || > + network_offset > transport_offset - nh_min_len) > return -EINVAL; > > - nh_min_len = skb_transport_offset(skb); > + nh_min_len = transport_offset; > p_off = nh_min_len + thlen; > if (!pskb_may_pull(skb, p_off)) > return -EINVAL; > @@ -206,9 +248,11 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > > static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian) > + bool little_endian, > + int network_offset) > { > - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type); > + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type, > + network_offset); > } > > /* This function must be called after virtio_net_hdr_from_skb(). */ > @@ -287,7 +331,7 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > return 0; > } > > -static inline unsigned int virtio_l3min(bool is_ipv6) > +static inline int virtio_l3min(bool is_ipv6) > { > return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); > } > @@ -297,18 +341,19 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *vhdr, > bool tnl_hdr_negotiated, > bool tnl_csum_negotiated, > - bool little_endian) > + bool little_endian, int network_offset) > { > const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr; > - unsigned int inner_nh, outer_th, inner_th; > - unsigned int inner_l3min, outer_l3min; > u8 gso_inner_type, gso_tunnel_type; > bool outer_isv6, inner_isv6; > + int inner_nh, outer_th, inner_th; > + int inner_l3min, outer_l3min; > int ret; > > gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; > if (!gso_tunnel_type) > - return virtio_net_hdr_to_skb(skb, hdr, little_endian); > + return virtio_net_hdr_to_skb(skb, hdr, little_endian, > + network_offset); > > /* Tunnel not supported/negotiated, but the hdr asks for it. */ > if (!tnl_hdr_negotiated) > @@ -332,19 +377,22 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; > inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; > inner_l3min = virtio_l3min(inner_isv6); > - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); > + outer_l3min = virtio_l3min(outer_isv6); > > inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); > inner_nh = le16_to_cpu(vhdr->inner_nh_offset); > outer_th = le16_to_cpu(vhdr->outer_th_offset); > - if (outer_th < outer_l3min || > + if (network_offset < 0 || > + outer_th < outer_l3min || > + network_offset > outer_th - outer_l3min || > inner_nh < outer_th + sizeof(struct udphdr) || > inner_th < inner_nh + inner_l3min) > return -EINVAL; > > /* Let the basic parsing deal with plain GSO features. */ > ret = __virtio_net_hdr_to_skb(skb, hdr, true, > - hdr->gso_type & ~gso_tunnel_type); > + hdr->gso_type & ~gso_tunnel_type, > + network_offset); > if (ret) > return ret; > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 50cae32ae..04c80e23d 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) > } > > if (has_vnet_hdr) { > - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { > + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb))) { > tp_len = -EINVAL; > goto tpacket_error; > } > @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) > packet_parse_headers(skb, sock); > > if (vnet_hdr_sz) { > - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); > + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb)); > if (err) > goto out_free; > len += vnet_hdr_sz; > -- > 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-21 22:11 ` Michael S. Tsirkin 2026-09-21 22:18 ` Michael S. Tsirkin @ 2026-09-21 22:44 ` Michael S. Tsirkin 2026-09-22 0:55 ` Paulos Yibelo 2026-09-24 8:54 ` netdev-bot+sashiko 3 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2026-09-21 22:44 UTC (permalink / raw) To: Paulos Yibelo Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel On Sun, Sep 20, 2026 at 10:53:40PM -0400, Paulos Yibelo wrote: > __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than > an estimated minimum network-header length. Its input offsets are relative > to skb->data. > > Using skb_network_offset() here is unsafe. TUN/TAP, virtio-net, and UML > parse a received virtio header before skb->network_header is established. > On an skb with headroom, the resulting negative offset enlarges the > apparent distance to the transport header and can admit a checksum start > inside the network header. > > Pass the data-relative L3 offset to the converter explicitly. IFF_TUN uses > zero, AF_PACKET supplies its established network offset, and Ethernet > receive paths parse Ethernet and nested VLAN headers with > skb_header_pointer(), without changing skb state. Use the same origin for > tunnel-offset validation, and make UML propagate conversion failures. > > This does not require a virtual-machine guest. A TUN or TAP device with > virtio-net header support is sufficient to reach these paths. > > Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.") > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ > Cc: stable@vger.kernel.org > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> > --- > Changes in v5: > - Replace the not-yet-established skb network-header offset with an > explicit data-relative L3 origin. > - Cover all in-tree callers, including Ethernet/VLAN receive paths, > tunnel metadata, and UML error propagation. > - Drop the prior Acked-by and Reviewed-by tags because the code changed. > > Changes in v4: > - State that a TUN device is sufficient and no guest is required, as > noted by Michael S. Tsirkin. > > Changes in v3: > - Keep the network-relative comparison on one line for readability, as > requested by David Ahern. > > Changes in v2: > - Make nh_min_len an int and remove the casts, as suggested by Michael S. > Tsirkin. > > arch/um/drivers/vector_transports.c | 10 +++- > drivers/net/tun_vnet.h | 28 ++++++++++- > drivers/net/virtio_net.c | 8 ++- > include/linux/virtio_net.h | 76 +++++++++++++++++++++++------ > net/packet/af_packet.c | 6 ++- > 5 files changed, 106 insertions(+), 22 deletions(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index ddd127ee9..79bc05fc6 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -197,6 +197,7 @@ static int raw_verify_header( > uint8_t *header, struct sk_buff *skb, struct vector_private *vp) > { > struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; > + int network_offset; > > if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && > (vp->req_size != 65536)) { > @@ -209,8 +210,13 @@ static int raw_verify_header( > if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) > return 1; > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > - return 0; > + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian(), > + network_offset); > } > > static bool get_uint_param( > diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h > index f4c652b1f..1c83c359d 100644 > --- a/drivers/net/tun_vnet.h > +++ b/drivers/net/tun_vnet.h > @@ -177,10 +177,27 @@ static inline int tun_vnet_hdr_put(int sz, struct iov_iter *iter, > return __tun_vnet_hdr_put(sz, 0, iter, hdr); > } > > +static inline int > +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) > +{ > + if ((flags & TUN_TYPE_MASK) != IFF_TAP) > + return 0; > + > + return virtio_net_hdr_get_l3_offset(skb, hdr); > +} > + > static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb, > const struct virtio_net_hdr *hdr) > { > - return virtio_net_hdr_to_skb(skb, hdr, tun_vnet_is_little_endian(flags)); > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr); > + > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, hdr, > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > /* > @@ -199,10 +216,17 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features, > struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *hdr) > { > + const struct virtio_net_hdr *vnet_hdr = (const struct virtio_net_hdr *)hdr; > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, vnet_hdr); > + > + if (network_offset < 0) > + return network_offset; > + > return virtio_net_hdr_tnl_to_skb(skb, hdr, > features & NETIF_F_GSO_UDP_TUNNEL, > features & NETIF_F_GSO_UDP_TUNNEL_CSUM, > - tun_vnet_is_little_endian(flags)); > + tun_vnet_is_little_endian(flags), > + network_offset); > } > > static inline int tun_vnet_hdr_from_skb(unsigned int flags, > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index e34c52d05..059eeb18e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2502,6 +2502,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > { > struct virtio_net_common_hdr *hdr; > struct net_device *dev = vi->dev; > + int network_offset; > > hdr = skb_vnet_common_hdr(skb); > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > @@ -2515,9 +2516,12 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > goto frame_err; > } > > - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > + network_offset = virtio_net_hdr_get_l3_offset(skb, &hdr->hdr); > + if (network_offset < 0 || > + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > vi->rx_tnl_csum, > - virtio_is_little_endian(vi->vdev))) { > + virtio_is_little_endian(vi->vdev), > + network_offset)) { > net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", > dev->name, hdr->hdr.gso_type, > hdr->hdr.gso_size, hdr->hdr.flags, > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index c381b916c..a4c005796 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -48,11 +48,49 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, > return 0; > } > > +/* > + * Return the L3 offset of an Ethernet frame starting at skb->data. > + * The offset is unused without NEEDS_CSUM, so avoid parsing and return zero. > + */ > +static inline int > +virtio_net_hdr_get_l3_offset(const struct sk_buff *skb, > + const struct virtio_net_hdr *hdr) > +{ > + unsigned int parse_depth = VLAN_MAX_DEPTH; > + const struct ethhdr *eth; > + struct ethhdr ethbuf; > + __be16 protocol; > + int depth = ETH_HLEN; > + > + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) > + return 0; > + > + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); > + if (!eth) > + return -EINVAL; > + > + protocol = eth->h_proto; > + while (eth_type_vlan(protocol)) { > + const struct vlan_hdr *vh; > + struct vlan_hdr vhdr; > + > + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); > + if (!vh || !--parse_depth) > + return -EINVAL; > + > + protocol = vh->h_vlan_encapsulated_proto; > + depth += VLAN_HLEN; > + } > + > + return depth; > +} > + > static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian, u8 hdr_gso_type) > + bool little_endian, u8 hdr_gso_type, > + int network_offset) > { > - unsigned int nh_min_len = sizeof(struct iphdr); > + int nh_min_len = sizeof(struct iphdr); So why is this IPv4 specific assumption still here? Confused. > unsigned int gso_type = 0; > unsigned int thlen = 0; > unsigned int p_off = 0; > @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > + int transport_offset; > > if (!pskb_may_pull(skb, needed)) > return -EINVAL; > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > - if (skb_transport_offset(skb) < nh_min_len) > + > + transport_offset = skb_transport_offset(skb); > + if (transport_offset < nh_min_len || network_offset < 0 || > + network_offset > transport_offset - nh_min_len) > return -EINVAL; > > - nh_min_len = skb_transport_offset(skb); > + nh_min_len = transport_offset; > p_off = nh_min_len + thlen; > if (!pskb_may_pull(skb, p_off)) > return -EINVAL; > @@ -206,9 +248,11 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > > static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr *hdr, > - bool little_endian) > + bool little_endian, > + int network_offset) > { > - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type); > + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type, > + network_offset); > } > > /* This function must be called after virtio_net_hdr_from_skb(). */ > @@ -287,7 +331,7 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, > return 0; > } > > -static inline unsigned int virtio_l3min(bool is_ipv6) > +static inline int virtio_l3min(bool is_ipv6) > { > return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); > } > @@ -297,18 +341,19 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > const struct virtio_net_hdr_v1_hash_tunnel *vhdr, > bool tnl_hdr_negotiated, > bool tnl_csum_negotiated, > - bool little_endian) > + bool little_endian, int network_offset) > { > const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr; > - unsigned int inner_nh, outer_th, inner_th; > - unsigned int inner_l3min, outer_l3min; > u8 gso_inner_type, gso_tunnel_type; > bool outer_isv6, inner_isv6; > + int inner_nh, outer_th, inner_th; > + int inner_l3min, outer_l3min; > int ret; > > gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; > if (!gso_tunnel_type) > - return virtio_net_hdr_to_skb(skb, hdr, little_endian); > + return virtio_net_hdr_to_skb(skb, hdr, little_endian, > + network_offset); > > /* Tunnel not supported/negotiated, but the hdr asks for it. */ > if (!tnl_hdr_negotiated) > @@ -332,19 +377,22 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; > inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; > inner_l3min = virtio_l3min(inner_isv6); > - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); > + outer_l3min = virtio_l3min(outer_isv6); > > inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); > inner_nh = le16_to_cpu(vhdr->inner_nh_offset); > outer_th = le16_to_cpu(vhdr->outer_th_offset); > - if (outer_th < outer_l3min || > + if (network_offset < 0 || > + outer_th < outer_l3min || > + network_offset > outer_th - outer_l3min || > inner_nh < outer_th + sizeof(struct udphdr) || > inner_th < inner_nh + inner_l3min) > return -EINVAL; > > /* Let the basic parsing deal with plain GSO features. */ > ret = __virtio_net_hdr_to_skb(skb, hdr, true, > - hdr->gso_type & ~gso_tunnel_type); > + hdr->gso_type & ~gso_tunnel_type, > + network_offset); > if (ret) > return ret; > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 50cae32ae..04c80e23d 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) > } > > if (has_vnet_hdr) { > - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { > + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb))) { > tp_len = -EINVAL; > goto tpacket_error; > } Will this do the right thing for ETH_P_IP sockets? even with vlan tags in the frame? > @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) > packet_parse_headers(skb, sock); > > if (vnet_hdr_sz) { > - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); > + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb)); > if (err) > goto out_free; > len += vnet_hdr_sz; > -- > 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 22:44 ` Michael S. Tsirkin @ 2026-09-22 0:55 ` Paulos Yibelo 0 siblings, 0 replies; 14+ messages in thread From: Paulos Yibelo @ 2026-09-22 0:55 UTC (permalink / raw) To: Michael S. Tsirkin Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 16262 bytes --] Thanks, I’ve addressed these points in v6 and will post the full revised series shortly when the timing gate opens. On Mon, Sep 21, 2026 at 6:44 PM Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Sep 20, 2026 at 10:53:40PM -0400, Paulos Yibelo wrote: > > __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than > > an estimated minimum network-header length. Its input offsets are > relative > > to skb->data. > > > > Using skb_network_offset() here is unsafe. TUN/TAP, virtio-net, and UML > > parse a received virtio header before skb->network_header is established. > > On an skb with headroom, the resulting negative offset enlarges the > > apparent distance to the transport header and can admit a checksum start > > inside the network header. > > > > Pass the data-relative L3 offset to the converter explicitly. IFF_TUN > uses > > zero, AF_PACKET supplies its established network offset, and Ethernet > > receive paths parse Ethernet and nested VLAN headers with > > skb_header_pointer(), without changing skb state. Use the same origin for > > tunnel-offset validation, and make UML propagate conversion failures. > > > > This does not require a virtual-machine guest. A TUN or TAP device with > > virtio-net header support is sufficient to reach these paths. > > > > Fixes: 49d14b54a527 ("net: test for not too small csum_start in > virtio_net_hdr_to_skb()") > > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO > tunneling.") > > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> > > Link: > https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ > > Cc: stable@vger.kernel.org > > Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> > > --- > > Changes in v5: > > - Replace the not-yet-established skb network-header offset with an > > explicit data-relative L3 origin. > > - Cover all in-tree callers, including Ethernet/VLAN receive paths, > > tunnel metadata, and UML error propagation. > > - Drop the prior Acked-by and Reviewed-by tags because the code changed. > > > > Changes in v4: > > - State that a TUN device is sufficient and no guest is required, as > > noted by Michael S. Tsirkin. > > > > Changes in v3: > > - Keep the network-relative comparison on one line for readability, as > > requested by David Ahern. > > > > Changes in v2: > > - Make nh_min_len an int and remove the casts, as suggested by Michael S. > > Tsirkin. > > > > arch/um/drivers/vector_transports.c | 10 +++- > > drivers/net/tun_vnet.h | 28 ++++++++++- > > drivers/net/virtio_net.c | 8 ++- > > include/linux/virtio_net.h | 76 +++++++++++++++++++++++------ > > net/packet/af_packet.c | 6 ++- > > 5 files changed, 106 insertions(+), 22 deletions(-) > > > > diff --git a/arch/um/drivers/vector_transports.c > b/arch/um/drivers/vector_transports.c > > index ddd127ee9..79bc05fc6 100644 > > --- a/arch/um/drivers/vector_transports.c > > +++ b/arch/um/drivers/vector_transports.c > > @@ -197,6 +197,7 @@ static int raw_verify_header( > > uint8_t *header, struct sk_buff *skb, struct vector_private *vp) > > { > > struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; > > + int network_offset; > > > > if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && > > (vp->req_size != 65536)) { > > @@ -209,8 +210,13 @@ static int raw_verify_header( > > if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) > > return 1; > > > > - virtio_net_hdr_to_skb(skb, vheader, > virtio_legacy_is_little_endian()); > > - return 0; > > + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); > > + if (network_offset < 0) > > + return network_offset; > > + > > + return virtio_net_hdr_to_skb(skb, vheader, > > + virtio_legacy_is_little_endian(), > > + network_offset); > > } > > > > static bool get_uint_param( > > diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h > > index f4c652b1f..1c83c359d 100644 > > --- a/drivers/net/tun_vnet.h > > +++ b/drivers/net/tun_vnet.h > > @@ -177,10 +177,27 @@ static inline int tun_vnet_hdr_put(int sz, struct > iov_iter *iter, > > return __tun_vnet_hdr_put(sz, 0, iter, hdr); > > } > > > > +static inline int > > +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff > *skb, > > + const struct virtio_net_hdr *hdr) > > +{ > > + if ((flags & TUN_TYPE_MASK) != IFF_TAP) > > + return 0; > > + > > + return virtio_net_hdr_get_l3_offset(skb, hdr); > > +} > > + > > static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct > sk_buff *skb, > > const struct virtio_net_hdr *hdr) > > { > > - return virtio_net_hdr_to_skb(skb, hdr, > tun_vnet_is_little_endian(flags)); > > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr); > > + > > + if (network_offset < 0) > > + return network_offset; > > + > > + return virtio_net_hdr_to_skb(skb, hdr, > > + tun_vnet_is_little_endian(flags), > > + network_offset); > > } > > > > /* > > @@ -199,10 +216,17 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, > netdev_features_t features, > > struct sk_buff *skb, > > const struct virtio_net_hdr_v1_hash_tunnel *hdr) > > { > > + const struct virtio_net_hdr *vnet_hdr = (const struct > virtio_net_hdr *)hdr; > > + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, > vnet_hdr); > > + > > + if (network_offset < 0) > > + return network_offset; > > + > > return virtio_net_hdr_tnl_to_skb(skb, hdr, > > features & NETIF_F_GSO_UDP_TUNNEL, > > features & NETIF_F_GSO_UDP_TUNNEL_CSUM, > > - tun_vnet_is_little_endian(flags)); > > + tun_vnet_is_little_endian(flags), > > + network_offset); > > } > > > > static inline int tun_vnet_hdr_from_skb(unsigned int flags, > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index e34c52d05..059eeb18e 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -2502,6 +2502,7 @@ static void virtnet_receive_done(struct > virtnet_info *vi, struct receive_queue * > > { > > struct virtio_net_common_hdr *hdr; > > struct net_device *dev = vi->dev; > > + int network_offset; > > > > hdr = skb_vnet_common_hdr(skb); > > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > > @@ -2515,9 +2516,12 @@ static void virtnet_receive_done(struct > virtnet_info *vi, struct receive_queue * > > goto frame_err; > > } > > > > - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > > + network_offset = virtio_net_hdr_get_l3_offset(skb, &hdr->hdr); > > + if (network_offset < 0 || > > + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > > vi->rx_tnl_csum, > > - virtio_is_little_endian(vi->vdev))) { > > + virtio_is_little_endian(vi->vdev), > > + network_offset)) { > > net_warn_ratelimited("%s: bad gso: type: %x, size: %u, > flags %x tunnel %d tnl csum %d\n", > > dev->name, hdr->hdr.gso_type, > > hdr->hdr.gso_size, hdr->hdr.flags, > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > > index c381b916c..a4c005796 100644 > > --- a/include/linux/virtio_net.h > > +++ b/include/linux/virtio_net.h > > @@ -48,11 +48,49 @@ static inline int virtio_net_hdr_set_proto(struct > sk_buff *skb, > > return 0; > > } > > > > +/* > > + * Return the L3 offset of an Ethernet frame starting at skb->data. > > + * The offset is unused without NEEDS_CSUM, so avoid parsing and return > zero. > > + */ > > +static inline int > > +virtio_net_hdr_get_l3_offset(const struct sk_buff *skb, > > + const struct virtio_net_hdr *hdr) > > +{ > > + unsigned int parse_depth = VLAN_MAX_DEPTH; > > + const struct ethhdr *eth; > > + struct ethhdr ethbuf; > > + __be16 protocol; > > + int depth = ETH_HLEN; > > + > > + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) > > + return 0; > > + > > + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); > > + if (!eth) > > + return -EINVAL; > > + > > + protocol = eth->h_proto; > > + while (eth_type_vlan(protocol)) { > > + const struct vlan_hdr *vh; > > + struct vlan_hdr vhdr; > > + > > + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); > > + if (!vh || !--parse_depth) > > + return -EINVAL; > > + > > + protocol = vh->h_vlan_encapsulated_proto; > > + depth += VLAN_HLEN; > > + } > > + > > + return depth; > > +} > > + > > static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > > const struct virtio_net_hdr *hdr, > > - bool little_endian, u8 > hdr_gso_type) > > + bool little_endian, u8 > hdr_gso_type, > > + int network_offset) > > { > > - unsigned int nh_min_len = sizeof(struct iphdr); > > + int nh_min_len = sizeof(struct iphdr); > > > So why is this IPv4 specific assumption still here? Confused. > > > > unsigned int gso_type = 0; > > unsigned int thlen = 0; > > unsigned int p_off = 0; > > @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct > sk_buff *skb, > > u32 start = __virtio16_to_cpu(little_endian, > hdr->csum_start); > > u32 off = __virtio16_to_cpu(little_endian, > hdr->csum_offset); > > u32 needed = start + max_t(u32, thlen, off + > sizeof(__sum16)); > > + int transport_offset; > > > > if (!pskb_may_pull(skb, needed)) > > return -EINVAL; > > > > if (!skb_partial_csum_set(skb, start, off)) > > return -EINVAL; > > - if (skb_transport_offset(skb) < nh_min_len) > > + > > + transport_offset = skb_transport_offset(skb); > > + if (transport_offset < nh_min_len || network_offset < 0 || > > + network_offset > transport_offset - nh_min_len) > > return -EINVAL; > > > > - nh_min_len = skb_transport_offset(skb); > > + nh_min_len = transport_offset; > > p_off = nh_min_len + thlen; > > if (!pskb_may_pull(skb, p_off)) > > return -EINVAL; > > @@ -206,9 +248,11 @@ static inline int __virtio_net_hdr_to_skb(struct > sk_buff *skb, > > > > static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > > const struct virtio_net_hdr *hdr, > > - bool little_endian) > > + bool little_endian, > > + int network_offset) > > { > > - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, > hdr->gso_type); > > + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, > hdr->gso_type, > > + network_offset); > > } > > > > /* This function must be called after virtio_net_hdr_from_skb(). */ > > @@ -287,7 +331,7 @@ static inline int virtio_net_hdr_from_skb(const > struct sk_buff *skb, > > return 0; > > } > > > > -static inline unsigned int virtio_l3min(bool is_ipv6) > > +static inline int virtio_l3min(bool is_ipv6) > > { > > return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); > > } > > @@ -297,18 +341,19 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > > const struct virtio_net_hdr_v1_hash_tunnel *vhdr, > > bool tnl_hdr_negotiated, > > bool tnl_csum_negotiated, > > - bool little_endian) > > + bool little_endian, int network_offset) > > { > > const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr > *)vhdr; > > - unsigned int inner_nh, outer_th, inner_th; > > - unsigned int inner_l3min, outer_l3min; > > u8 gso_inner_type, gso_tunnel_type; > > bool outer_isv6, inner_isv6; > > + int inner_nh, outer_th, inner_th; > > + int inner_l3min, outer_l3min; > > int ret; > > > > gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; > > if (!gso_tunnel_type) > > - return virtio_net_hdr_to_skb(skb, hdr, little_endian); > > + return virtio_net_hdr_to_skb(skb, hdr, little_endian, > > + network_offset); > > > > /* Tunnel not supported/negotiated, but the hdr asks for it. */ > > if (!tnl_hdr_negotiated) > > @@ -332,19 +377,22 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > > outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; > > inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; > > inner_l3min = virtio_l3min(inner_isv6); > > - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); > > + outer_l3min = virtio_l3min(outer_isv6); > > > > inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); > > inner_nh = le16_to_cpu(vhdr->inner_nh_offset); > > outer_th = le16_to_cpu(vhdr->outer_th_offset); > > - if (outer_th < outer_l3min || > > + if (network_offset < 0 || > > + outer_th < outer_l3min || > > + network_offset > outer_th - outer_l3min || > > inner_nh < outer_th + sizeof(struct udphdr) || > > inner_th < inner_nh + inner_l3min) > > return -EINVAL; > > > > /* Let the basic parsing deal with plain GSO features. */ > > ret = __virtio_net_hdr_to_skb(skb, hdr, true, > > - hdr->gso_type & ~gso_tunnel_type); > > + hdr->gso_type & ~gso_tunnel_type, > > + network_offset); > > if (ret) > > return ret; > > > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > > index 50cae32ae..04c80e23d 100644 > > --- a/net/packet/af_packet.c > > +++ b/net/packet/af_packet.c > > @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, > struct msghdr *msg) > > } > > > > if (has_vnet_hdr) { > > - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, > vio_le())) { > > + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > > + > skb_network_offset(skb))) { > > tp_len = -EINVAL; > > goto tpacket_error; > > } > > Will this do the right thing for ETH_P_IP sockets? even with vlan tags > in the frame? > > > > @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct > msghdr *msg, size_t len) > > packet_parse_headers(skb, sock); > > > > if (vnet_hdr_sz) { > > - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); > > + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > > + skb_network_offset(skb)); > > if (err) > > goto out_free; > > len += vnet_hdr_sz; > > -- > > 2.46.0 > > -- PGP Key ID: 53911E7F16446D4C :) [-- Attachment #1.2: Type: text/html, Size: 20832 bytes --] [-- Attachment #2: 0002-ip-reject-partial-checksums-covering-network-headers.patch --] [-- Type: application/octet-stream, Size: 6192 bytes --] From fb99a82e3f13e6a2800ac1df0ebbcd6bc525df00 Mon Sep 17 00:00:00 2001 From: Paulos Yibelo <habte.yibelo@gmail.com> Date: Sun, 20 Sep 2026 22:16:49 -0400 Subject: [PATCH net v6 2/2] ip: reject partial checksums covering network headers ip_do_fragment() and nf_br_ip_fragment() complete a CHECKSUM_PARTIAL skb before reading the IPv4 header length. ip6_fragment() and br_ip6_fragment() complete one after parsing the IPv6 header chain. A virtualization interface can supply a checksum start which, after link-layer removal, still points inside that parsed network header. skb_checksum_help() then writes the completed checksum into header bytes the stack has already consumed. For IPv4, changing iph->ihl after routing and validation can make fragmentation copy beyond the skb's logical linear head into transmitted options. A negative checksum-start offset is rejected by skb_checksum_help(), but only after a WARN_ONCE which can panic a panic_on_warn system. Validate the checksum start against the parsed header length before completing it. For IPv4, read and validate IHL first, retain it, and reacquire iph after skb_checksum_help() in both implementations. For IPv6, use the length returned by ip6_find_1stfragopt() in both implementations. Compare the signed checksum-start offset with the bounded signed header length so integer promotion cannot bypass either boundary. Fixes: dbd3393c56a8 ("ipv4: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") Fixes: 405c92f7a541 ("ipv6: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") Fixes: 3c171f496ef5 ("netfilter: bridge: add connection tracking system") Fixes: 764dd163ac92 ("netfilter: nf_conntrack_bridge: add support for IPv6") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Link: https://lore.kernel.org/netdev/20260920004733.6473-3-habte.yibelo@gmail.com/ Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> --- net/bridge/netfilter/nf_conntrack_bridge.c | 21 +++++++++++++++----- net/ipv4/ip_output.c | 23 ++++++++++++++++------ net/ipv6/ip6_output.c | 12 ++++++++--- net/ipv6/netfilter.c | 12 ++++++++--- 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c index 7ecb8a26b..d81ed8692 100644 --- a/net/bridge/netfilter/nf_conntrack_bridge.c +++ b/net/bridge/netfilter/nf_conntrack_bridge.c @@ -38,18 +38,29 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk, struct iphdr *iph; int err = 0; - /* for offloaded checksums cleanup checksum before fragmentation */ - if (skb->ip_summed == CHECKSUM_PARTIAL && - (err = skb_checksum_help(skb))) + iph = ip_hdr(skb); + hlen = iph->ihl * 4; + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { + err = -EINVAL; goto blackhole; + } - iph = ip_hdr(skb); + /* Complete offloaded checksums only after the validated IP header. */ + if (skb->ip_summed == CHECKSUM_PARTIAL) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto blackhole; + } + err = skb_checksum_help(skb); + if (err) + goto blackhole; + iph = ip_hdr(skb); + } /* * Setup starting values */ - hlen = iph->ihl * 4; frag_max_size -= hlen; ll_rs = LL_RESERVED_SPACE(skb->dev); mtu = skb->dev->mtu; diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index a24cc8ee1..fa6a74d20 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -770,16 +770,28 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, struct ip_frag_state state; int err = 0; - /* for offloaded checksums cleanup checksum before fragmentation */ - if (skb->ip_summed == CHECKSUM_PARTIAL && - (err = skb_checksum_help(skb))) - goto fail; - /* * Point into the IP datagram header. */ iph = ip_hdr(skb); + hlen = iph->ihl * 4; + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { + err = -EINVAL; + goto fail; + } + + /* Complete offloaded checksums only after the validated IP header. */ + if (skb->ip_summed == CHECKSUM_PARTIAL) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto fail; + } + err = skb_checksum_help(skb); + if (err) + goto fail; + iph = ip_hdr(skb); + } mtu = ip_skb_dst_mtu(sk, skb); if (IPCB(skb)->frag_max_size && IPCB(skb)->frag_max_size < mtu) @@ -789,7 +801,6 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, * Setup starting values. */ - hlen = iph->ihl * 4; if (mtu < hlen + 8) { err = -EMSGSIZE; goto fail; diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 550965058..d157b6ade 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -942,9 +942,15 @@ 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) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto fail; + } + 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 a7025ec87..da7ada12f 100644 --- a/net/ipv6/netfilter.c +++ b/net/ipv6/netfilter.c @@ -144,9 +144,15 @@ 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) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto blackhole; + } + err = skb_checksum_help(skb); + if (err) + goto blackhole; + } prevhdr = skb_network_header(skb) + nexthdr_offset; hroom = LL_RESERVED_SPACE(skb->dev); -- 2.46.0 [-- Attachment #3: 0001-net-validate-virtio-checksum-start-after-network-hea.patch --] [-- Type: application/octet-stream, Size: 14052 bytes --] From dfab3bad4732c4b97347421a0a3a4c690e912be4 Mon Sep 17 00:00:00 2001 From: Paulos Yibelo <habte.yibelo@gmail.com> Date: Mon, 21 Sep 2026 19:38:51 -0400 Subject: [PATCH net v6 1/2] net: validate virtio checksum start after network header __virtio_net_hdr_to_skb() checks a minimum network-header length for CHECKSUM_PARTIAL packets. Its checksum start is relative to skb->data, but some callers have not established skb->network_header when they convert the virtio header. Pass the data-relative L3 origin explicitly. Ethernet receive paths parse the frame and nested VLAN headers without changing skb state. AF_PACKET uses the frame's actual L3 origin even when the socket protocol is ETH_P_IP and the raw frame carries VLAN tags. Non-Ethernet AF_PACKET devices retain their established skb network offset. Also pass the actual L3 protocol so IPv6 packets use the 40-byte base header minimum even without TCPv6 GSO. IFF_TUN obtains that protocol from the packet before skb->protocol is set. Name the Ethernet parser accordingly, use the same origin for tunnel validation, and propagate conversion failures in UML. The bound remains a minimum; fragmentation paths separately validate the parsed IPv4 or IPv6 header length before completing a checksum. Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()") Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO tunneling.") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> --- arch/um/drivers/vector_transports.c | 13 ++++- drivers/net/tun_vnet.h | 52 ++++++++++++++++- drivers/net/virtio_net.c | 10 +++- include/linux/virtio_net.h | 87 ++++++++++++++++++++++++----- net/packet/af_packet.c | 24 +++++++- 5 files changed, 163 insertions(+), 23 deletions(-) diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c index ddd127ee9..5a9d8f49a 100644 --- a/arch/um/drivers/vector_transports.c +++ b/arch/um/drivers/vector_transports.c @@ -197,6 +197,8 @@ static int raw_verify_header( uint8_t *header, struct sk_buff *skb, struct vector_private *vp) { struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; + __be16 network_protocol; + int network_offset; if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && (vp->req_size != 65536)) { @@ -209,8 +211,14 @@ static int raw_verify_header( if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) return 1; - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); - return 0; + network_offset = virtio_net_hdr_eth_get_l3_offset(skb, vheader, + &network_protocol); + if (network_offset < 0) + return network_offset; + + return virtio_net_hdr_to_skb(skb, vheader, + virtio_legacy_is_little_endian(), + network_offset, network_protocol); } static bool get_uint_param( @@ -491,4 +499,3 @@ int build_transport_data(struct vector_private *vp) return build_bess_transport_data(vp); return 0; } - diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h index f4c652b1f..e004213fa 100644 --- a/drivers/net/tun_vnet.h +++ b/drivers/net/tun_vnet.h @@ -177,10 +177,49 @@ static inline int tun_vnet_hdr_put(int sz, struct iov_iter *iter, return __tun_vnet_hdr_put(sz, 0, iter, hdr); } +static inline int +tun_vnet_hdr_get_l3_offset(unsigned int flags, const struct sk_buff *skb, + const struct virtio_net_hdr *hdr, + __be16 *network_protocol) +{ + if ((flags & TUN_TYPE_MASK) != IFF_TAP) { + u8 version, first_byte; + const u8 *first; + + *network_protocol = 0; + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) + return 0; + + first = skb_header_pointer(skb, 0, sizeof(first_byte), + &first_byte); + if (!first) + return -EINVAL; + + version = *first >> 4; + if (version == 4) + *network_protocol = htons(ETH_P_IP); + else if (version == 6) + *network_protocol = htons(ETH_P_IPV6); + return 0; + } + + return virtio_net_hdr_eth_get_l3_offset(skb, hdr, + network_protocol); +} + static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb, const struct virtio_net_hdr *hdr) { - return virtio_net_hdr_to_skb(skb, hdr, tun_vnet_is_little_endian(flags)); + __be16 network_protocol; + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, hdr, + &network_protocol); + + if (network_offset < 0) + return network_offset; + + return virtio_net_hdr_to_skb(skb, hdr, + tun_vnet_is_little_endian(flags), + network_offset, network_protocol); } /* @@ -199,10 +238,19 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features, struct sk_buff *skb, const struct virtio_net_hdr_v1_hash_tunnel *hdr) { + const struct virtio_net_hdr *vnet_hdr = (const struct virtio_net_hdr *)hdr; + __be16 network_protocol; + int network_offset = tun_vnet_hdr_get_l3_offset(flags, skb, vnet_hdr, + &network_protocol); + + if (network_offset < 0) + return network_offset; + return virtio_net_hdr_tnl_to_skb(skb, hdr, features & NETIF_F_GSO_UDP_TUNNEL, features & NETIF_F_GSO_UDP_TUNNEL_CSUM, - tun_vnet_is_little_endian(flags)); + tun_vnet_is_little_endian(flags), + network_offset, network_protocol); } static inline int tun_vnet_hdr_from_skb(unsigned int flags, diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index e34c52d05..8e93dad28 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2502,6 +2502,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * { struct virtio_net_common_hdr *hdr; struct net_device *dev = vi->dev; + __be16 network_protocol; + int network_offset; hdr = skb_vnet_common_hdr(skb); if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) @@ -2515,9 +2517,13 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * goto frame_err; } - if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, + network_offset = virtio_net_hdr_eth_get_l3_offset(skb, &hdr->hdr, + &network_protocol); + if (network_offset < 0 || + virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, vi->rx_tnl_csum, - virtio_is_little_endian(vi->vdev))) { + virtio_is_little_endian(vi->vdev), + network_offset, network_protocol)) { net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", dev->name, hdr->hdr.gso_type, hdr->hdr.gso_size, hdr->hdr.flags, diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index c381b916c..b89d821a0 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -48,16 +48,61 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, return 0; } +/* + * Return the L3 offset and protocol of an Ethernet frame starting at skb->data. + * The offset is unused without NEEDS_CSUM, so avoid parsing and return zero. + */ +static inline int +virtio_net_hdr_eth_get_l3_offset(const struct sk_buff *skb, + const struct virtio_net_hdr *hdr, + __be16 *network_protocol) +{ + unsigned int parse_depth = VLAN_MAX_DEPTH; + const struct ethhdr *eth; + struct ethhdr ethbuf; + __be16 protocol; + int depth = ETH_HLEN; + + *network_protocol = 0; + if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) + return 0; + + eth = skb_header_pointer(skb, 0, sizeof(ethbuf), ðbuf); + if (!eth) + return -EINVAL; + + protocol = eth->h_proto; + while (eth_type_vlan(protocol)) { + const struct vlan_hdr *vh; + struct vlan_hdr vhdr; + + vh = skb_header_pointer(skb, depth, sizeof(vhdr), &vhdr); + if (!vh || !--parse_depth) + return -EINVAL; + + protocol = vh->h_vlan_encapsulated_proto; + depth += VLAN_HLEN; + } + + *network_protocol = protocol; + return depth; +} + static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, const struct virtio_net_hdr *hdr, - bool little_endian, u8 hdr_gso_type) + bool little_endian, u8 hdr_gso_type, + int network_offset, + __be16 network_protocol) { - unsigned int nh_min_len = sizeof(struct iphdr); + int nh_min_len = sizeof(struct iphdr); unsigned int gso_type = 0; unsigned int thlen = 0; unsigned int p_off = 0; unsigned int ip_proto; + if (network_protocol == htons(ETH_P_IPV6)) + nh_min_len = sizeof(struct ipv6hdr); + if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) { switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) { case VIRTIO_NET_HDR_GSO_TCPV4: @@ -98,16 +143,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); + int transport_offset; if (!pskb_may_pull(skb, needed)) return -EINVAL; if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; - if (skb_transport_offset(skb) < nh_min_len) + + transport_offset = skb_transport_offset(skb); + if (transport_offset < nh_min_len || network_offset < 0 || + network_offset > transport_offset - nh_min_len) return -EINVAL; - nh_min_len = skb_transport_offset(skb); + nh_min_len = transport_offset; p_off = nh_min_len + thlen; if (!pskb_may_pull(skb, p_off)) return -EINVAL; @@ -206,9 +255,12 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, const struct virtio_net_hdr *hdr, - bool little_endian) + bool little_endian, + int network_offset, + __be16 network_protocol) { - return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type); + return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type, + network_offset, network_protocol); } /* This function must be called after virtio_net_hdr_from_skb(). */ @@ -287,7 +339,7 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb, return 0; } -static inline unsigned int virtio_l3min(bool is_ipv6) +static inline int virtio_l3min(bool is_ipv6) { return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr); } @@ -297,18 +349,20 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, const struct virtio_net_hdr_v1_hash_tunnel *vhdr, bool tnl_hdr_negotiated, bool tnl_csum_negotiated, - bool little_endian) + bool little_endian, int network_offset, + __be16 network_protocol) { const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr; - unsigned int inner_nh, outer_th, inner_th; - unsigned int inner_l3min, outer_l3min; u8 gso_inner_type, gso_tunnel_type; bool outer_isv6, inner_isv6; + int inner_nh, outer_th, inner_th; + int inner_l3min, outer_l3min; int ret; gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL; if (!gso_tunnel_type) - return virtio_net_hdr_to_skb(skb, hdr, little_endian); + return virtio_net_hdr_to_skb(skb, hdr, little_endian, + network_offset, network_protocol); /* Tunnel not supported/negotiated, but the hdr asks for it. */ if (!tnl_hdr_negotiated) @@ -332,19 +386,24 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, outer_isv6 = gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6; inner_isv6 = gso_inner_type == VIRTIO_NET_HDR_GSO_TCPV6; inner_l3min = virtio_l3min(inner_isv6); - outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6); + outer_l3min = virtio_l3min(outer_isv6); + if (network_protocol == htons(ETH_P_IPV6)) + outer_l3min = sizeof(struct ipv6hdr); inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start); inner_nh = le16_to_cpu(vhdr->inner_nh_offset); outer_th = le16_to_cpu(vhdr->outer_th_offset); - if (outer_th < outer_l3min || + if (network_offset < 0 || + outer_th < outer_l3min || + network_offset > outer_th - outer_l3min || inner_nh < outer_th + sizeof(struct udphdr) || inner_th < inner_nh + inner_l3min) return -EINVAL; /* Let the basic parsing deal with plain GSO features. */ ret = __virtio_net_hdr_to_skb(skb, hdr, true, - hdr->gso_type & ~gso_tunnel_type); + hdr->gso_type & ~gso_tunnel_type, + network_offset, network_protocol); if (ret) return ret; diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 50cae32ae..0b37d5474 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2550,6 +2550,26 @@ static void tpacket_destruct_skb(struct sk_buff *skb) sock_wfree(skb); } +static int packet_vnet_hdr_to_skb(struct sk_buff *skb, + const struct virtio_net_hdr *vnet_hdr) +{ + __be16 network_protocol = 0; + int network_offset; + + if (skb->dev->type == ARPHRD_ETHER) { + network_offset = virtio_net_hdr_eth_get_l3_offset(skb, vnet_hdr, + &network_protocol); + } else { + network_offset = skb_network_offset(skb); + network_protocol = skb->protocol; + } + if (network_offset < 0) + return network_offset; + + return virtio_net_hdr_to_skb(skb, vnet_hdr, vio_le(), + network_offset, network_protocol); +} + static int __packet_snd_vnet_parse(struct virtio_net_hdr *vnet_hdr, size_t len) { if ((vnet_hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && @@ -2901,7 +2921,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) } if (has_vnet_hdr) { - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { + if (packet_vnet_hdr_to_skb(skb, &vnet_hdr)) { tp_len = -EINVAL; goto tpacket_error; } @@ -3103,7 +3123,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) packet_parse_headers(skb, sock); if (vnet_hdr_sz) { - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); + err = packet_vnet_hdr_to_skb(skb, &vnet_hdr); if (err) goto out_free; len += vnet_hdr_sz; base-commit: 1e24c4f2ee44be0eee94092b5d13cbdb4bdf0d60 -- 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 1/2] net: validate virtio checksum start after network header 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo ` (2 preceding siblings ...) 2026-09-21 22:44 ` Michael S. Tsirkin @ 2026-09-24 8:54 ` netdev-bot+sashiko 3 siblings, 0 replies; 14+ messages in thread From: netdev-bot+sashiko @ 2026-09-24 8:54 UTC (permalink / raw) To: habte.yibelo Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, mst, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 1 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] The two AF_PACKET call sites pass skb_network_offset(skb) as the new `network_offset` argument, while every other converted caller… - [Medium] In arch/um/drivers/vector_transports.c:raw_verify_header(), the newly added virtio_net_hdr_get_l3_offset() parse and the now-… -- net: validate virtio checksum start after network header This adds an explicit data-relative L3 offset argument to virtio_net_hdr_to_skb() and virtio_net_hdr_tnl_to_skb() so that a CHECKSUM_PARTIAL start landing inside the network header is rejected. A new helper virtio_net_hdr_get_l3_offset() parses Ethernet and nested VLAN headers with skb_header_pointer(), and all in-tree callers are converted. Link: https://lore.kernel.org/netdev/20260920004733.6473-2-habte.yibelo@gmail.com/ > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index ddd127ee9678..79bc05fc6f73 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -197,6 +197,7 @@ static int raw_verify_header( > uint8_t *header, struct sk_buff *skb, struct vector_private *vp) > { > struct virtio_net_hdr *vheader = (struct virtio_net_hdr *) header; > + int network_offset; > > if ((vheader->gso_type != VIRTIO_NET_HDR_GSO_NONE) && > (vp->req_size != 65536)) { > @@ -209,8 +210,13 @@ static int raw_verify_header( > if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0) > return 1; > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > - return 0; > + network_offset = virtio_net_hdr_get_l3_offset(skb, vheader); > + if (network_offset < 0) > + return network_offset; > + > + return virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian(), > + network_offset); > } [Severity: Medium] Is the skb passed to raw_verify_header() already trimmed to the received frame length at this point? prep_skb() in arch/um/drivers/vector_kern.c sets the length from the receive buffer capacity, before the recvmsg/recvmmsg: skb_put(result, vp->max_packet); result->data_len = len - vp->max_packet; result->len += len - vp->max_packet; and both receive loops call the verify callback before the trim: vector_legacy_rx(): header_check = vp->verify_header( vp->header_rxbuffer, skb, vp); if (header_check < 0) { ... return 0; } } pskb_trim(skb, pkt_len - vp->rx_header_size); vector_mmsg_rx() has the same ordering. The only guarantee before the callback is pkt_len > vp->header_size, i.e. one payload byte. So can the Ethernet and VLAN bytes that virtio_net_hdr_get_l3_offset() reads at offsets 0..13 and beyond be bytes recvmsg never wrote, i.e. the uninitialised remainder of the allocation, making network_offset and the accept/reject decision depend on stale buffer contents whenever the received frame is shorter than the parsed span? Along the same lines, the length checks inside __virtio_net_hdr_to_skb() see skb->len as capacity (up to vp->req_size, 65536 with GSO) rather than the received length, for instance: if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS) return -EINVAL; Since raw_verify_header() now returns the converter's error instead of discarding it, does this turn the length mismatch into frames dropped with rx_encaps_errors, where previously the return value was ignored? Would it be better to move the pskb_trim() ahead of the verify_header() call? > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index c381b916c1b5..a4c005796acb 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h [ ... ] > @@ -98,16 +136,20 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > + int transport_offset; > > if (!pskb_may_pull(skb, needed)) > return -EINVAL; > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > - if (skb_transport_offset(skb) < nh_min_len) > + > + transport_offset = skb_transport_offset(skb); > + if (transport_offset < nh_min_len || network_offset < 0 || > + network_offset > transport_offset - nh_min_len) > return -EINVAL; [ ... ] > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 50cae32ae269..04c80e23d7e2 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -2901,7 +2901,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) > } > > if (has_vnet_hdr) { > - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { > + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb))) { > tp_len = -EINVAL; > goto tpacket_error; > } > @@ -3103,7 +3104,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) > packet_parse_headers(skb, sock); > > if (vnet_hdr_sz) { > - err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); > + err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le(), > + skb_network_offset(skb)); > if (err) > goto out_free; > len += vnet_hdr_sz; [Severity: High] Is skb_network_offset(skb) a strong enough origin here? Every other converted caller derives the offset from the frame bytes via virtio_net_hdr_get_l3_offset(), while these two use the network header AF_PACKET established itself. In packet_parse_headers() the VLAN adjustment is gated on the socket-supplied protocol: if (likely(skb->dev->type == ARPHRD_ETHER) && eth_type_vlan(skb->protocol) && vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0) skb_set_network_header(skb, depth); For SOCK_RAW with an explicit sll_protocol, skb->protocol is the value the sender passed, not the frame's EtherType (dev_parse_header_protocol() runs only when skb->protocol is 0 or ETH_P_ALL). So for a sender that binds with sll_protocol = htons(ETH_P_IP) but writes an 802.1Q-tagged frame, network_offset stays 14 while the real IPv4 header starts at 18. With csum_start = 34 and nh_min_len = 20, the new test becomes: transport_offset = 34; 34 < 20 -> false 14 > 34 - 20 -> 14 > 14 -> false and the header is accepted, after skb_partial_csum_set() has already set ip_summed = CHECKSUM_PARTIAL and transport_header = 34, which is 16 bytes inside the IPv4 header spanning 18..37. Is that not exactly the case the commit message says is now rejected? Two related cases in the same path: packet_parse_headers() silently ignores a failed vlan_get_protocol_and_depth() (more nested tags than that parser accepts), leaving network_offset at 14 for a deeper real L3 header. In packet_snd(), the SOCK_RAW short-packet branch resets the network header while skb->data still points at the link header: } else if (reserve) { skb_reserve(skb, -reserve); if (len < reserve + sizeof(struct ipv6hdr) && dev->min_header_len != hard_header_len) skb_reset_network_header(skb); } which yields network_offset == 0, and the new bound then reduces to the pre-patch csum_start >= 20 check. Would using virtio_net_hdr_get_l3_offset() at both AF_PACKET call sites, as the TAP and virtio-net paths do, keep the origin consistent? Note the follow-up commit in this area, "ip: reject partial checksums covering network headers", only touches ip_do_fragment(), ip6_fragment(), nf_br_ip_fragment() and br_ip6_fragment(), so it does not close this AF_PACKET path later in the series. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921025341.44846-1-habte.yibelo%40gmail.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v5 2/2] ip: reject partial checksums covering network headers 2026-09-21 2:53 ` [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers Paulos Yibelo 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo @ 2026-09-21 2:53 ` Paulos Yibelo 2026-09-24 8:54 ` netdev-bot+sashiko 1 sibling, 1 reply; 14+ messages in thread From: Paulos Yibelo @ 2026-09-21 2:53 UTC (permalink / raw) To: netdev Cc: richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, mst, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel ip_do_fragment() and nf_br_ip_fragment() complete a CHECKSUM_PARTIAL skb before reading the IPv4 header length. ip6_fragment() and br_ip6_fragment() complete one after parsing the IPv6 header chain. A virtualization interface can supply a checksum start which, after link-layer removal, still points inside that parsed network header. skb_checksum_help() then writes the completed checksum into header bytes the stack has already consumed. For IPv4, changing iph->ihl after routing and validation can make fragmentation copy beyond the skb's logical linear head into transmitted options. A negative checksum-start offset is rejected by skb_checksum_help(), but only after a WARN_ONCE which can panic a panic_on_warn system. Validate the checksum start against the parsed header length before completing it. For IPv4, read and validate IHL first, retain it, and reacquire iph after skb_checksum_help() in both implementations. For IPv6, use the length returned by ip6_find_1stfragopt() in both implementations. Compare the signed checksum-start offset with the bounded signed header length so integer promotion cannot bypass either boundary. Fixes: dbd3393c56a8 ("ipv4: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") Fixes: 405c92f7a541 ("ipv6: add defensive check for CHECKSUM_PARTIAL skbs in ip_fragment") Fixes: 3c171f496ef5 ("netfilter: bridge: add connection tracking system") Fixes: 764dd163ac92 ("netfilter: nf_conntrack_bridge: add support for IPv6") Reported-by: Paulos Yibelo <habte.yibelo@gmail.com> Link: https://lore.kernel.org/netdev/20260920004733.6473-3-habte.yibelo@gmail.com/ Cc: stable@vger.kernel.org Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com> --- Changes in v5: - Compare the checksum-start offset and IPv4 header length as signed values. - Add parsed-header checks to the IPv4/IPv6 output and bridge-netfilter fragmentation paths. - Drop the prior Acked-by and Reviewed-by tags because the code changed. Changes in v4: - State that a TUN device is sufficient and no guest is required, as noted by Michael S. Tsirkin. Changes in v3: - No code changes. Changes in v2: - No code changes. net/bridge/netfilter/nf_conntrack_bridge.c | 21 +++++++++++++++----- net/ipv4/ip_output.c | 23 ++++++++++++++++------ net/ipv6/ip6_output.c | 12 ++++++++--- net/ipv6/netfilter.c | 12 ++++++++--- 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c index 7ecb8a26b..d81ed8692 100644 --- a/net/bridge/netfilter/nf_conntrack_bridge.c +++ b/net/bridge/netfilter/nf_conntrack_bridge.c @@ -38,18 +38,29 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk, struct iphdr *iph; int err = 0; - /* for offloaded checksums cleanup checksum before fragmentation */ - if (skb->ip_summed == CHECKSUM_PARTIAL && - (err = skb_checksum_help(skb))) + iph = ip_hdr(skb); + hlen = iph->ihl * 4; + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { + err = -EINVAL; goto blackhole; + } - iph = ip_hdr(skb); + /* Complete offloaded checksums only after the validated IP header. */ + if (skb->ip_summed == CHECKSUM_PARTIAL) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto blackhole; + } + err = skb_checksum_help(skb); + if (err) + goto blackhole; + iph = ip_hdr(skb); + } /* * Setup starting values */ - hlen = iph->ihl * 4; frag_max_size -= hlen; ll_rs = LL_RESERVED_SPACE(skb->dev); mtu = skb->dev->mtu; diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index a24cc8ee1..fa6a74d20 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -770,16 +770,28 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, struct ip_frag_state state; int err = 0; - /* for offloaded checksums cleanup checksum before fragmentation */ - if (skb->ip_summed == CHECKSUM_PARTIAL && - (err = skb_checksum_help(skb))) - goto fail; - /* * Point into the IP datagram header. */ iph = ip_hdr(skb); + hlen = iph->ihl * 4; + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { + err = -EINVAL; + goto fail; + } + + /* Complete offloaded checksums only after the validated IP header. */ + if (skb->ip_summed == CHECKSUM_PARTIAL) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto fail; + } + err = skb_checksum_help(skb); + if (err) + goto fail; + iph = ip_hdr(skb); + } mtu = ip_skb_dst_mtu(sk, skb); if (IPCB(skb)->frag_max_size && IPCB(skb)->frag_max_size < mtu) @@ -789,7 +801,6 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, * Setup starting values. */ - hlen = iph->ihl * 4; if (mtu < hlen + 8) { err = -EMSGSIZE; goto fail; diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 550965058..d157b6ade 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -942,9 +942,15 @@ 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) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto fail; + } + 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 a7025ec87..da7ada12f 100644 --- a/net/ipv6/netfilter.c +++ b/net/ipv6/netfilter.c @@ -144,9 +144,15 @@ 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) { + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { + err = -EINVAL; + goto blackhole; + } + err = skb_checksum_help(skb); + if (err) + goto blackhole; + } prevhdr = skb_network_header(skb) + nexthdr_offset; hroom = LL_RESERVED_SPACE(skb->dev); -- 2.46.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v5 2/2] ip: reject partial checksums covering network headers 2026-09-21 2:53 ` [PATCH net v5 2/2] ip: reject partial checksums covering network headers Paulos Yibelo @ 2026-09-24 8:54 ` netdev-bot+sashiko 0 siblings, 0 replies; 14+ messages in thread From: netdev-bot+sashiko @ 2026-09-24 8:54 UTC (permalink / raw) To: habte.yibelo Cc: netdev, richard, anton.ivanov, johannes, willemdebruijn.kernel, jasowangio, mst, eperezma, xuanzhuo, andrew+netdev, pablo, fw, phil, razor, idosch, dsahern, davem, edumazet, kuba, pabeni, horms, linux-um, virtualization, netfilter-devel, coreteam, bridge, linux-kernel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 0 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] Incomplete fix: the checksum-start validation is added only at the four fragmentation entry points (ip_do_fragment(),… -- ip: reject partial checksums covering network headers This adds a check that the CHECKSUM_PARTIAL start offset lies at or after the parsed network header before calling skb_checksum_help() in the four fragmentation entry points ip_do_fragment(), nf_br_ip_fragment(), ip6_fragment() and br_ip6_fragment(). For IPv4 the IHL is read and validated first and iph is reacquired after the checksum is completed. Link: https://lore.kernel.org/netdev/20260920004733.6473-3-habte.yibelo@gmail.com/ > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > index a24cc8ee11d3..fa6a74d205e2 100644 > --- a/net/ipv4/ip_output.c > +++ b/net/ipv4/ip_output.c > @@ -770,16 +770,28 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, > struct ip_frag_state state; > int err = 0; > > - /* for offloaded checksums cleanup checksum before fragmentation */ > - if (skb->ip_summed == CHECKSUM_PARTIAL && > - (err = skb_checksum_help(skb))) > - goto fail; > - > /* > * Point into the IP datagram header. > */ > > iph = ip_hdr(skb); > + hlen = iph->ihl * 4; > + if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) { > + err = -EINVAL; > + goto fail; > + } > + > + /* Complete offloaded checksums only after the validated IP header. */ > + if (skb->ip_summed == CHECKSUM_PARTIAL) { > + if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) { > + err = -EINVAL; > + goto fail; > + } > + err = skb_checksum_help(skb); > + if (err) > + goto fail; > + iph = ip_hdr(skb); > + } [Severity: High] The guard is added at the four fragmentation entry points, but the offending csum_start stays on the skb. Is the bug class named in the subject still reachable for packets that are not fragmented? The generic transmit path completes the same checksum without any comparable check: net/core/dev.c:validate_xmit_skb() { ... if (skb_csum_hwoffload_help(skb, features)) goto out_kfree_skb; ... } net/core/dev.c:skb_csum_hwoffload_help() { ... sw_checksum: return skb_checksum_help(skb); } and so does xfrm_output(): net/xfrm/xfrm_output.c:xfrm_output() { ... if (skb->ip_summed == CHECKSUM_PARTIAL) { err = skb_checksum_help(skb); ... } For the IPv6 case described in the changelog, a tun/tap or virtio client can supply csum_start=34 and csum_offset=6 on a non-GSO frame. In __virtio_net_hdr_to_skb() nh_min_len is sizeof(struct iphdr) for non-GSO frames, so the ingress check passes with network_offset==ETH_HLEN and transport_offset==34: include/linux/virtio_net.h:__virtio_net_hdr_to_skb() { ... transport_offset = skb_transport_offset(skb); if (transport_offset < nh_min_len || network_offset < 0 || network_offset > transport_offset - nh_min_len) return -EINVAL; ... } After eth_type_trans() pulls ETH_HLEN, skb_checksum_start_offset(skb) is 20, which is inside the 40 byte IPv6 header. If the frame fits the MTU it never reaches ip6_fragment(), and skb_csum_hwoffload_help() takes the sw_checksum path on a device without NETIF_F_HW_CSUM because skb_network_header_len(skb) is 20 rather than sizeof(struct ipv6hdr). Can the fold then land at offset 26, inside ipv6_hdr()->daddr, after routing and netfilter have already validated the original destination? The IPv4 equivalent is a start offset of 20 with ihl*4 up to 60, overwriting option bytes of the transmitted packet. The same applies to the WARN_ONCE motivation in the changelog: A negative checksum-start offset is rejected by skb_checksum_help(), but only after a WARN_ONCE which can panic a panic_on_warn system. In skb_checksum_help() the signed offset is compared with the unsigned skb_headlen(), so a negative value promotes and takes the DO_ONCE_LITE(skb_dump) plus WARN_ONCE() branch: net/core/dev.c:skb_checksum_help() { ... offset = skb_checksum_start_offset(skb); ret = -EINVAL; if (unlikely(offset >= skb_headlen(skb))) { DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false); WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n", ... } Any skb that could carry such an offset into ip_do_fragment() also reaches skb_csum_hwoffload_help() unchanged when the packet fits the MTU, so is the panic_on_warn case still open there? Would it be preferable to validate where csum_start is established or invalidated, that is in skb_partial_csum_set() and __virtio_net_hdr_to_skb() relative to the eth_type_trans() pull, or inside skb_checksum_help() itself, rather than open coding the same test in four consumers? Since the patch carries its own Fixes: tags and Cc: stable and will be backported on its own, could the changelog state whether an ingress side companion fix exists, or that the remaining skb_checksum_help() callers are knowingly left unguarded? As written, the subject "reject partial checksums covering network headers" reads as a general property while only the fragmentation paths are covered. The memory safety part specific to fragmentation, iph->ihl being mutated after routing and then used by the fragmentation loops, does look closed by this change. [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921025341.44846-1-habte.yibelo%40gmail.com ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-09-24 8:54 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-20 0:47 [PATCH net v4 0/2] net: prevent partial checksums from modifying IPv4 headers Paulos Yibelo 2026-09-20 0:47 ` [PATCH net v4 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-20 1:11 ` David Ahern 2026-09-20 0:47 ` [PATCH net v4 2/2] ipv4: reject partial checksums covering the IP header Paulos Yibelo 2026-09-20 1:12 ` David Ahern 2026-09-21 2:53 ` [PATCH net v5 0/2] net: prevent partial checksums from modifying network headers Paulos Yibelo 2026-09-21 2:53 ` [PATCH net v5 1/2] net: validate virtio checksum start after network header Paulos Yibelo 2026-09-21 22:11 ` Michael S. Tsirkin 2026-09-21 22:18 ` Michael S. Tsirkin 2026-09-21 22:44 ` Michael S. Tsirkin 2026-09-22 0:55 ` Paulos Yibelo 2026-09-24 8:54 ` netdev-bot+sashiko 2026-09-21 2:53 ` [PATCH net v5 2/2] ip: reject partial checksums covering network headers Paulos Yibelo 2026-09-24 8:54 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®