* [PATCH net-next 0/4] more virtio-net csum hardening
@ 2026-10-13 10:58 Michael S. Tsirkin
2026-10-13 10:58 ` [PATCH net-next 1/4] net: add ip4_parse() Michael S. Tsirkin
` (5 more replies)
0 siblings, 6 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 10:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky
Allowing untrusted hardware to specify csum start/offset for
CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
outside the actual checksum, and depending on the setup, the networking
stack could be looking at the packet both before and after the edit, and
seeing different results.
Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
completing checksum") validated all known instances where this
causes the most grief, but the real fix is to make sure
that either only actual checksum fields are ever edited,
or the checksum is completed before the net stack looks at it,
so it is always consistent.
This basically does this: checksum at ingress, except in a narrow list
of specified known good cases.
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Michael S. Tsirkin (4):
net: add ip4_parse()
net: use ip4_parse() in receive validation
net: resolve uncommon partial checksums at ingress
net: account for ingress-resolved virtio net hdr state
drivers/net/tap.c | 4 +-
drivers/net/tun.c | 44 +++++++-
drivers/net/virtio_net.c | 2 +-
include/linux/ip.h | 24 +++++
include/linux/virtio_net.h | 161 ++++++++++++++++++++++++----
net/bridge/br_netfilter_hooks.c | 15 +--
net/ipv4/ip_input.c | 13 +--
net/ipv4/netfilter/nf_reject_ipv4.c | 13 +--
net/packet/af_packet.c | 4 +-
9 files changed, 228 insertions(+), 52 deletions(-)
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net-next 1/4] net: add ip4_parse()
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
@ 2026-10-13 10:58 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 2/4] net: use ip4_parse() in receive validation Michael S. Tsirkin
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 10:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
Multiple places in the net stack open-code the same minimal IPv4 header
validation: pull the fixed header, check version 4 and IHL >= 5 (where 5
is the fixed header size in 32-bit units) - and locate the transport
header at IHL * 4 past the network header.
Add a helper for doing that: it returns the validated header and
the transport header offset, or NULL if truncated or malformed.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: LLM
---
include/linux/ip.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/include/linux/ip.h b/include/linux/ip.h
index d11c25f5030a0..98012f6d0efb5 100644
--- a/include/linux/ip.h
+++ b/include/linux/ip.h
@@ -56,4 +56,28 @@ static inline void iph_set_totlen(struct iphdr *iph, unsigned int len)
{
iph->tot_len = len <= IP_MAX_MTU ? htons(len) : 0;
}
+
+/*
+ * Pull and minimally validate the IPv4 header at @nhoff.
+ * Returns the header and stores the transport header offset in
+ * @thoff, or NULL if truncated or malformed. Only the fixed header
+ * is guaranteed linear; callers that need the options or the
+ * transport header must pull further.
+ */
+static inline struct iphdr *ip4_parse(struct sk_buff *skb,
+ unsigned int nhoff,
+ unsigned int *thoff)
+{
+ struct iphdr *iph;
+
+ if (!pskb_may_pull(skb, nhoff + sizeof(struct iphdr)))
+ return NULL;
+
+ iph = (struct iphdr *)(skb->data + nhoff);
+ if (iph->version != 4 || iph->ihl < sizeof(struct iphdr) / sizeof(u32))
+ return NULL;
+
+ *thoff = nhoff + iph->ihl * sizeof(u32);
+ return iph;
+}
#endif /* _LINUX_IP_H */
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net-next 2/4] net: use ip4_parse() in receive validation
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
2026-10-13 10:58 ` [PATCH net-next 1/4] net: add ip4_parse() Michael S. Tsirkin
@ 2026-10-13 10:58 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress Michael S. Tsirkin
` (3 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 10:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
Nikolay Aleksandrov, Ido Schimmel, Simon Horman, David Ahern,
netfilter-devel, coreteam, bridge
Convert the sites that pull and do version and ihl checks on the ipv4
header and validate the packet transport offset to use ip4_parse.
Sites that validate a read-only copy instead of pulling - the nft
and nft_inner header_pointer validators, ip_check_defrag()'s
skb_copy_bits, the flow dissector - stay open-coded for now.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: LLM
---
net/bridge/br_netfilter_hooks.c | 15 +++++----------
net/ipv4/ip_input.c | 13 +++++--------
net/ipv4/netfilter/nf_reject_ipv4.c | 13 +++++--------
3 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 0a394e5f43916..e89da0e415426 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -206,18 +206,13 @@ static inline void nf_bridge_pull_encap_header_rcsum(struct sk_buff *skb)
static int br_validate_ipv4(struct net *net, struct sk_buff *skb)
{
const struct iphdr *iph;
- u32 len;
+ u32 len, thoff;
- if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+ iph = ip4_parse(skb, 0, &thoff);
+ if (!iph)
goto inhdr_error;
- iph = ip_hdr(skb);
-
- /* Basic sanity checks */
- if (iph->ihl < 5 || iph->version != 4)
- goto inhdr_error;
-
- if (!pskb_may_pull(skb, iph->ihl*4))
+ if (!pskb_may_pull(skb, thoff))
goto inhdr_error;
iph = ip_hdr(skb);
@@ -228,7 +223,7 @@ static int br_validate_ipv4(struct net *net, struct sk_buff *skb)
if (skb->len < len) {
__IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
goto drop;
- } else if (len < (iph->ihl*4))
+ } else if (len < thoff)
goto inhdr_error;
if (pskb_trim_rcsum(skb, len)) {
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 9860178752b8c..5adf01763e857 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -500,7 +500,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
{
const struct iphdr *iph;
int drop_reason;
- u32 len;
+ u32 len, thoff;
/* When the interface is in promisc. mode, drop all the crap
* that it receives, do not try to analyse it.
@@ -520,10 +520,6 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
}
drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
- if (!pskb_may_pull(skb, sizeof(struct iphdr)))
- goto inhdr_error;
-
- iph = ip_hdr(skb);
/*
* RFC1122: 3.2.1.2 MUST silently discard any IP frame that fails the checksum.
@@ -536,7 +532,8 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
* 4. Doesn't have a bogus length
*/
- if (iph->ihl < 5 || iph->version != 4)
+ iph = ip4_parse(skb, 0, &thoff);
+ if (!iph)
goto inhdr_error;
BUILD_BUG_ON(IPSTATS_MIB_ECT1PKTS != IPSTATS_MIB_NOECTPKTS + INET_ECN_ECT_1);
@@ -546,7 +543,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
IPSTATS_MIB_NOECTPKTS + (iph->tos & INET_ECN_MASK),
max_t(unsigned short, 1, skb_shinfo(skb)->gso_segs));
- if (!pskb_may_pull(skb, iph->ihl*4))
+ if (!pskb_may_pull(skb, thoff))
goto inhdr_error;
iph = ip_hdr(skb);
@@ -559,7 +556,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
drop_reason = SKB_DROP_REASON_PKT_TOO_SMALL;
__IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
goto drop;
- } else if (len < (iph->ihl*4))
+ } else if (len < thoff)
goto inhdr_error;
/* Our transport medium may have padded the buffer out. Now we know it
diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index 59ec465a9df98..42d639854ddf1 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -25,22 +25,19 @@ nf_reject_ip_tcphdr_get(struct sk_buff *oldskb,
static int nf_reject_iphdr_validate(struct sk_buff *skb)
{
struct iphdr *iph;
- u32 len;
+ u32 len, thoff;
- if (!pskb_may_pull(skb, sizeof(struct iphdr)))
- return 0;
-
- iph = ip_hdr(skb);
- if (iph->ihl < 5 || iph->version != 4)
+ iph = ip4_parse(skb, 0, &thoff);
+ if (!iph)
return 0;
len = ntohs(iph->tot_len);
if (skb->len < len)
return 0;
- else if (len < (iph->ihl*4))
+ else if (len < thoff)
return 0;
- if (!pskb_may_pull(skb, iph->ihl*4))
+ if (!pskb_may_pull(skb, thoff))
return 0;
return 1;
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
2026-10-13 10:58 ` [PATCH net-next 1/4] net: add ip4_parse() Michael S. Tsirkin
2026-10-13 10:58 ` [PATCH net-next 2/4] net: use ip4_parse() in receive validation Michael S. Tsirkin
@ 2026-10-13 10:58 ` Michael S. Tsirkin
2026-10-13 11:06 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
` (2 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 10:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
A virtio net header can place CHECKSUM_PARTIAL at any in-packet
offset. If that state is retained, later checksum completion can
modify the packet after protocol processing has made decisions from it.
Let's fix that: let's validate csum_start and csum_offset and only keep
CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or
UDP header, while handling VLAN headers, IPv4 options and IPv6 extension
headers. Complete other non-GSO checksums immediately, before any
protocol processing runs.
We can't easily "complete the checksum" for GSO we don't recognize - GSO
assumes knowledge of the protocol. We could segment ignoring the
checksum then do whatever csum_start and csum_offset say, but that
seems like an overkill for something that should happen.
UDP tunnel headers get the same handling on the inner header.
Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: LLM
---
include/linux/virtio_net.h | 151 ++++++++++++++++++++++++++++++++-----
1 file changed, 133 insertions(+), 18 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index d6466f96cdd00..b181183118b3a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -5,7 +5,11 @@
#include <linux/if_vlan.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
+#include <linux/netdevice.h>
#include <linux/udp.h>
+#include <net/gso.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
#include <net/tcp.h>
#include <uapi/linux/tcp.h>
#include <uapi/linux/virtio_net.h>
@@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
return 0;
}
+/* Does checksum metadata match a known TCP or UDP header? */
+static inline bool
+virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
+ unsigned int nhoff, __be16 proto)
+{
+ unsigned int thoff;
+ int ip_proto;
+
+ if (proto == htons(ETH_P_IP)) {
+ const struct iphdr *iph = ip4_parse(skb, nhoff, &thoff);
+
+ if (!iph || ip_is_fragment(iph))
+ return false;
+ ip_proto = iph->protocol;
+ } else if (proto == htons(ETH_P_IPV6)) {
+ const struct ipv6hdr *iph;
+ int flags = 0;
+
+ if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
+ return false;
+ iph = (const void *)(skb->data + nhoff);
+ if (iph->version != 6)
+ return false;
+ thoff = nhoff + sizeof(*iph);
+ ip_proto = iph->nexthdr;
+ if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_UDP) {
+ thoff = nhoff;
+ ip_proto = ipv6_find_hdr(skb, &thoff, -1, NULL,
+ &flags);
+ if (ip_proto < 0 || flags & IP6_FH_F_FRAG)
+ return false;
+ }
+ } else {
+ return false;
+ }
+
+ if (skb_checksum_start_offset(skb) != thoff)
+ return false;
+
+ switch (ip_proto) {
+ case IPPROTO_TCP:
+ if (skb->csum_offset != offsetof(struct tcphdr, check) ||
+ !pskb_may_pull(skb, thoff + sizeof(struct tcphdr)))
+ return false;
+ break;
+ case IPPROTO_UDP:
+ if (skb->csum_offset != offsetof(struct udphdr, check) ||
+ !pskb_may_pull(skb, thoff + sizeof(struct udphdr)))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+ case VIRTIO_NET_HDR_GSO_NONE:
+ return true;
+ case VIRTIO_NET_HDR_GSO_TCPV4:
+ return proto == htons(ETH_P_IP) && ip_proto == IPPROTO_TCP;
+ case VIRTIO_NET_HDR_GSO_TCPV6:
+ return proto == htons(ETH_P_IPV6) && ip_proto == IPPROTO_TCP;
+ case VIRTIO_NET_HDR_GSO_UDP:
+ case VIRTIO_NET_HDR_GSO_UDP_L4:
+ return ip_proto == IPPROTO_UDP;
+ default:
+ return false;
+ }
+}
+
+static inline bool
+virtio_net_csum_partial_ok(struct sk_buff *skb, u8 hdr_gso_type)
+{
+ unsigned int nhoff;
+ __be16 proto;
+
+ if (skb->dev->type == ARPHRD_ETHER) {
+ if (!pskb_may_pull(skb, ETH_HLEN))
+ return false;
+ nhoff = ETH_HLEN;
+ proto = eth_hdr(skb)->h_proto;
+ if (eth_type_vlan(proto)) {
+ proto = __vlan_get_protocol(skb, proto, &nhoff);
+ if (!proto)
+ return false;
+ }
+ } else {
+ nhoff = skb_network_offset(skb);
+ proto = skb->protocol;
+ }
+
+ return virtio_net_csum_partial_ok_at_offset(skb, hdr_gso_type, nhoff,
+ proto);
+}
+
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,
+ bool need_csum_check)
{
- unsigned int nh_min_len = sizeof(struct iphdr);
+ typeof(skb->transport_header) old_transport = skb->transport_header;
unsigned int gso_type = 0;
unsigned int thlen = 0;
unsigned int p_off = 0;
unsigned int ip_proto;
+ bool csum_partial_ok = true;
if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
@@ -69,7 +169,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
gso_type = SKB_GSO_TCPV6;
ip_proto = IPPROTO_TCP;
thlen = sizeof(struct tcphdr);
- nh_min_len = sizeof(struct ipv6hdr);
break;
case VIRTIO_NET_HDR_GSO_UDP:
gso_type = SKB_GSO_UDP;
@@ -104,11 +203,11 @@ 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)
- return -EINVAL;
+ if (need_csum_check)
+ csum_partial_ok =
+ virtio_net_csum_partial_ok(skb, hdr_gso_type);
- nh_min_len = skb_transport_offset(skb);
- p_off = nh_min_len + thlen;
+ p_off = skb_transport_offset(skb) + thlen;
if (!pskb_may_pull(skb, p_off))
return -EINVAL;
} else if (gso_type) {
@@ -158,8 +257,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
case SKB_GSO_UDP_L4:
if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
return -EINVAL;
- if (skb->csum_offset != offsetof(struct udphdr, check))
- return -EINVAL;
if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
return -EINVAL;
if (gso_type != SKB_GSO_UDP_L4)
@@ -167,10 +264,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
break;
case SKB_GSO_TCPV4:
case SKB_GSO_TCPV6:
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- skb->csum_offset != offsetof(struct tcphdr, check))
- return -EINVAL;
-
BUILD_BUG_ON(TCP_MIN_GSO_SIZE * GSO_MAX_SEGS < GSO_MAX_SIZE);
gso_size = max(gso_size, TCP_MIN_GSO_SIZE);
break;
@@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
}
}
+ if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
+ int err;
+
+ if (skb_is_gso(skb))
+ return -EINVAL;
+
+ err = skb_checksum_help(skb);
+ skb->transport_header = old_transport;
+ return err;
+ }
+
return 0;
}
@@ -198,7 +302,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
const struct virtio_net_hdr *hdr,
bool little_endian)
{
- 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, true);
}
/* This function must be called after virtio_net_hdr_from_skb(). */
@@ -293,6 +398,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
unsigned int inner_nh, outer_th, inner_th;
unsigned int inner_l3min, outer_l3min;
u8 gso_inner_type, gso_tunnel_type;
+ __be16 inner_proto;
bool outer_isv6, inner_isv6;
int ret;
@@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
/* 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, false);
if (ret)
return ret;
@@ -352,8 +458,8 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
return -EINVAL;
}
- skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
- htons(ETH_P_IP));
+ inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
+ skb_set_inner_protocol(skb, inner_proto);
if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
if (!tnl_csum_negotiated)
return -EINVAL;
@@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
skb->inner_mac_header = inner_nh + skb_headroom(skb);
skb->transport_header = outer_th + skb_headroom(skb);
skb->encapsulation = 1;
- return 0;
+
+ if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
+ inner_proto))
+ return 0;
+
+ if (skb_is_gso(skb))
+ return -EINVAL;
+
+ ret = skb_checksum_help(skb);
+ return ret ?: 1;
}
/* Checksum-related fields validation for the driver */
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
` (2 preceding siblings ...)
2026-10-13 10:58 ` [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress Michael S. Tsirkin
@ 2026-10-13 10:58 ` Michael S. Tsirkin
2026-10-13 17:45 ` Björn Töpel
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 11:05 ` [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
2026-10-13 14:41 ` Willem de Bruijn
5 siblings, 2 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 10:58 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell, Simon Horman
Ingress resolution of uncommon virtio net hdr checksum metadata
(software checksum completion at ingress) adds performance overhead but
is currently invisible: no counter tracks it, so a hypothetical workload
relying on uncommon metadata cannot be distinguished from none at all,
and packets quietly pay the host-CPU checksum overhead instead of offload.
Have virtio_net_hdr_to_skb() return 1 when it completes an uncommon
checksum in software, and flip every converter call site to test for
negative errno. Have tun count these events per device as
rx_csum_sw and expose the counter via ethtool -S.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Assisted-by: LLM
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
---
drivers/net/tap.c | 4 ++--
drivers/net/tun.c | 44 ++++++++++++++++++++++++++++++++++++--
drivers/net/virtio_net.c | 2 +-
include/linux/virtio_net.h | 12 +++++++++--
net/packet/af_packet.c | 4 ++--
5 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index ff67d99deb39e..73e8833944463 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -649,7 +649,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
if (vnet_hdr_len) {
err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr);
- if (err) {
+ if (err < 0) {
rcu_read_unlock();
drop_reason = SKB_DROP_REASON_DEV_HDR;
goto err_kfree;
@@ -1084,7 +1084,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
if (vnet_hdr_len) {
err = tun_vnet_hdr_to_skb(q->flags, skb, gso);
- if (err) {
+ if (err < 0) {
rcu_read_unlock();
goto err_kfree;
}
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 242899f7fd071..138ca8987dfc6 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -210,6 +210,7 @@ struct tun_struct {
u32 flow_count;
u32 rx_batched;
atomic_long_t rx_frame_errors;
+ atomic_long_t rx_csum_sw;
struct bpf_prog __rcu *xdp_prog;
struct tun_prog __rcu *steering_prog;
struct tun_prog __rcu *filter_prog;
@@ -1935,11 +1936,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
break;
}
- if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
+ err = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr);
+ if (err < 0) {
atomic_long_inc(&tun->rx_frame_errors);
err = -EINVAL;
goto free_skb;
}
+ if (err > 0)
+ atomic_long_inc(&tun->rx_csum_sw);
+ /* The status is consumed: keep err clear so later exit paths
+ * (XDP drop via unlock_frags) do not return it to write() as
+ * a short count.
+ */
+ err = 0;
if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
skb->protocol = eth_type_trans(skb, tun->dev);
@@ -2605,12 +2614,15 @@ static int tun_xdp_one(struct tun_struct *tun,
tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
skb->dev = tun->dev;
skb_set_network_header(skb, ETH_HLEN);
- if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
+ ret = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr);
+ if (ret < 0) {
atomic_long_inc(&tun->rx_frame_errors);
kfree_skb(skb);
ret = -EINVAL;
goto out;
}
+ if (ret > 0)
+ atomic_long_inc(&tun->rx_csum_sw);
skb->protocol = eth_type_trans(skb, tun->dev);
skb_reset_network_header(skb);
@@ -3739,7 +3751,35 @@ static void tun_get_channels(struct net_device *dev,
channels->max_combined = tun->flags & IFF_MULTI_QUEUE ? MAX_TAP_QUEUES : 1;
}
+#define TUN_STATS_LEN 1
+static const char tun_ethtool_strings[TUN_STATS_LEN][ETH_GSTRING_LEN] = {
+ "rx_csum_sw",
+};
+
+static int tun_get_sset_count(struct net_device *dev, int sset)
+{
+ return sset == ETH_SS_STATS ? TUN_STATS_LEN : -EOPNOTSUPP;
+}
+
+static void tun_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+ if (stringset == ETH_SS_STATS)
+ memcpy(data, tun_ethtool_strings,
+ sizeof(tun_ethtool_strings));
+}
+
+static void tun_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct tun_struct *tun = netdev_priv(dev);
+
+ data[0] = atomic_long_read(&tun->rx_csum_sw);
+}
+
static const struct ethtool_ops tun_ethtool_ops = {
+ .get_sset_count = tun_get_sset_count,
+ .get_strings = tun_get_strings,
+ .get_ethtool_stats = tun_get_ethtool_stats,
.supported_coalesce_params = ETHTOOL_COALESCE_RX_MAX_FRAMES,
.get_drvinfo = tun_get_drvinfo,
.get_msglevel = tun_get_msglevel,
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index daab43ac92ce4..2e6fd81df9c41 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2519,7 +2519,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
skb_set_network_header(skb, ETH_HLEN);
if (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)) < 0) {
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 b181183118b3a..d8447db58ac83 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -14,6 +14,14 @@
#include <uapi/linux/tcp.h>
#include <uapi/linux/virtio_net.h>
+/*
+ * virtio_net_hdr_to_skb() and virtio_net_hdr_tnl_to_skb() return 0 on
+ * success and a negative errno on failure. On success they may also
+ * return 1, indicating that an uncommon checksum was completed in
+ * software at ingress rather than deferred to the egress path;
+ * drivers can use this to account for such packets.
+ */
+
static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
{
switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
@@ -292,7 +300,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
err = skb_checksum_help(skb);
skb->transport_header = old_transport;
- return err;
+ return err ?: 1;
}
return 0;
@@ -441,7 +449,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
/* Let the basic parsing deal with plain GSO features. */
ret = __virtio_net_hdr_to_skb(skb, hdr, true,
hdr->gso_type & ~gso_tunnel_type, false);
- if (ret)
+ if (ret < 0)
return ret;
/* In case of USO, the inner protocol is still unknown and
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 52ae3d875e971..e6aec9ecfa994 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2913,7 +2913,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 (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()) < 0) {
tp_len = -EINVAL;
goto tpacket_error;
}
@@ -3136,7 +3136,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
if (vnet_hdr_sz) {
err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
- if (err)
+ if (err < 0)
goto out_free;
len += vnet_hdr_sz;
}
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
` (3 preceding siblings ...)
2026-10-13 10:58 ` [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
@ 2026-10-13 11:05 ` Michael S. Tsirkin
2026-10-13 18:51 ` Chris Arges
2026-10-13 14:41 ` Willem de Bruijn
5 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 11:05 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 06:58:28AM -0400, Michael S. Tsirkin wrote:
> Allowing untrusted hardware to specify csum start/offset for
> CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> outside the actual checksum, and depending on the setup, the networking
> stack could be looking at the packet both before and after the edit, and
> seeing different results.
>
> Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> completing checksum") validated all known instances where this
> causes the most grief, but the real fix is to make sure
> that either only actual checksum fields are ever edited,
> or the checksum is completed before the net stack looks at it,
> so it is always consistent.
>
> This basically does this: checksum at ingress, except in a narrow list
> of specified known good cases.
>
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
I forgot.
Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
Reported-by: Chris Arges <carges@cloudflare.com>
> Michael S. Tsirkin (4):
> net: add ip4_parse()
> net: use ip4_parse() in receive validation
> net: resolve uncommon partial checksums at ingress
> net: account for ingress-resolved virtio net hdr state
>
> drivers/net/tap.c | 4 +-
> drivers/net/tun.c | 44 +++++++-
> drivers/net/virtio_net.c | 2 +-
> include/linux/ip.h | 24 +++++
> include/linux/virtio_net.h | 161 ++++++++++++++++++++++++----
> net/bridge/br_netfilter_hooks.c | 15 +--
> net/ipv4/ip_input.c | 13 +--
> net/ipv4/netfilter/nf_reject_ipv4.c | 13 +--
> net/packet/af_packet.c | 4 +-
> 9 files changed, 228 insertions(+), 52 deletions(-)
>
> --
> MST
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 10:58 ` [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress Michael S. Tsirkin
@ 2026-10-13 11:06 ` Michael S. Tsirkin
2026-10-13 15:03 ` Willem de Bruijn
2026-10-14 10:59 ` netdev-bot+sashiko
1 sibling, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 11:06 UTC (permalink / raw)
To: linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 06:58:41AM -0400, Michael S. Tsirkin wrote:
> A virtio net header can place CHECKSUM_PARTIAL at any in-packet
> offset. If that state is retained, later checksum completion can
> modify the packet after protocol processing has made decisions from it.
>
> Let's fix that: let's validate csum_start and csum_offset and only keep
> CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or
> UDP header, while handling VLAN headers, IPv4 options and IPv6 extension
> headers. Complete other non-GSO checksums immediately, before any
> protocol processing runs.
>
> We can't easily "complete the checksum" for GSO we don't recognize - GSO
> assumes knowledge of the protocol. We could segment ignoring the
> checksum then do whatever csum_start and csum_offset say, but that
> seems like an overkill for something that should happen.
>
> UDP tunnel headers get the same handling on the inner header.
>
> Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.
>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: LLM
I forgot reported-by tags:
Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
Reported-by: Chris Arges <carges@cloudflare.com>
> ---
> include/linux/virtio_net.h | 151 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 133 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd00..b181183118b3a 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -5,7 +5,11 @@
> #include <linux/if_vlan.h>
> #include <linux/ip.h>
> #include <linux/ipv6.h>
> +#include <linux/netdevice.h>
> #include <linux/udp.h>
> +#include <net/gso.h>
> +#include <net/ip.h>
> +#include <net/ipv6.h>
> #include <net/tcp.h>
> #include <uapi/linux/tcp.h>
> #include <uapi/linux/virtio_net.h>
> @@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
> return 0;
> }
>
> +/* Does checksum metadata match a known TCP or UDP header? */
> +static inline bool
> +virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
> + unsigned int nhoff, __be16 proto)
> +{
> + unsigned int thoff;
> + int ip_proto;
> +
> + if (proto == htons(ETH_P_IP)) {
> + const struct iphdr *iph = ip4_parse(skb, nhoff, &thoff);
> +
> + if (!iph || ip_is_fragment(iph))
> + return false;
> + ip_proto = iph->protocol;
> + } else if (proto == htons(ETH_P_IPV6)) {
> + const struct ipv6hdr *iph;
> + int flags = 0;
> +
> + if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
> + return false;
> + iph = (const void *)(skb->data + nhoff);
> + if (iph->version != 6)
> + return false;
> + thoff = nhoff + sizeof(*iph);
> + ip_proto = iph->nexthdr;
> + if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_UDP) {
> + thoff = nhoff;
> + ip_proto = ipv6_find_hdr(skb, &thoff, -1, NULL,
> + &flags);
> + if (ip_proto < 0 || flags & IP6_FH_F_FRAG)
> + return false;
> + }
> + } else {
> + return false;
> + }
> +
> + if (skb_checksum_start_offset(skb) != thoff)
> + return false;
> +
> + switch (ip_proto) {
> + case IPPROTO_TCP:
> + if (skb->csum_offset != offsetof(struct tcphdr, check) ||
> + !pskb_may_pull(skb, thoff + sizeof(struct tcphdr)))
> + return false;
> + break;
> + case IPPROTO_UDP:
> + if (skb->csum_offset != offsetof(struct udphdr, check) ||
> + !pskb_may_pull(skb, thoff + sizeof(struct udphdr)))
> + return false;
> + break;
> + default:
> + return false;
> + }
> +
> + switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> + case VIRTIO_NET_HDR_GSO_NONE:
> + return true;
> + case VIRTIO_NET_HDR_GSO_TCPV4:
> + return proto == htons(ETH_P_IP) && ip_proto == IPPROTO_TCP;
> + case VIRTIO_NET_HDR_GSO_TCPV6:
> + return proto == htons(ETH_P_IPV6) && ip_proto == IPPROTO_TCP;
> + case VIRTIO_NET_HDR_GSO_UDP:
> + case VIRTIO_NET_HDR_GSO_UDP_L4:
> + return ip_proto == IPPROTO_UDP;
> + default:
> + return false;
> + }
> +}
> +
> +static inline bool
> +virtio_net_csum_partial_ok(struct sk_buff *skb, u8 hdr_gso_type)
> +{
> + unsigned int nhoff;
> + __be16 proto;
> +
> + if (skb->dev->type == ARPHRD_ETHER) {
> + if (!pskb_may_pull(skb, ETH_HLEN))
> + return false;
> + nhoff = ETH_HLEN;
> + proto = eth_hdr(skb)->h_proto;
> + if (eth_type_vlan(proto)) {
> + proto = __vlan_get_protocol(skb, proto, &nhoff);
> + if (!proto)
> + return false;
> + }
> + } else {
> + nhoff = skb_network_offset(skb);
> + proto = skb->protocol;
> + }
> +
> + return virtio_net_csum_partial_ok_at_offset(skb, hdr_gso_type, nhoff,
> + proto);
> +}
> +
> 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,
> + bool need_csum_check)
> {
> - unsigned int nh_min_len = sizeof(struct iphdr);
> + typeof(skb->transport_header) old_transport = skb->transport_header;
> unsigned int gso_type = 0;
> unsigned int thlen = 0;
> unsigned int p_off = 0;
> unsigned int ip_proto;
> + bool csum_partial_ok = true;
>
> if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> @@ -69,7 +169,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> gso_type = SKB_GSO_TCPV6;
> ip_proto = IPPROTO_TCP;
> thlen = sizeof(struct tcphdr);
> - nh_min_len = sizeof(struct ipv6hdr);
> break;
> case VIRTIO_NET_HDR_GSO_UDP:
> gso_type = SKB_GSO_UDP;
> @@ -104,11 +203,11 @@ 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)
> - return -EINVAL;
> + if (need_csum_check)
> + csum_partial_ok =
> + virtio_net_csum_partial_ok(skb, hdr_gso_type);
>
> - nh_min_len = skb_transport_offset(skb);
> - p_off = nh_min_len + thlen;
> + p_off = skb_transport_offset(skb) + thlen;
> if (!pskb_may_pull(skb, p_off))
> return -EINVAL;
> } else if (gso_type) {
> @@ -158,8 +257,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> case SKB_GSO_UDP_L4:
> if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> return -EINVAL;
> - if (skb->csum_offset != offsetof(struct udphdr, check))
> - return -EINVAL;
> if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
> return -EINVAL;
> if (gso_type != SKB_GSO_UDP_L4)
> @@ -167,10 +264,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> break;
> case SKB_GSO_TCPV4:
> case SKB_GSO_TCPV6:
> - if (skb->ip_summed == CHECKSUM_PARTIAL &&
> - skb->csum_offset != offsetof(struct tcphdr, check))
> - return -EINVAL;
> -
> BUILD_BUG_ON(TCP_MIN_GSO_SIZE * GSO_MAX_SEGS < GSO_MAX_SIZE);
> gso_size = max(gso_size, TCP_MIN_GSO_SIZE);
> break;
> @@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> }
> }
>
> + if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
> + int err;
> +
> + if (skb_is_gso(skb))
> + return -EINVAL;
> +
> + err = skb_checksum_help(skb);
> + skb->transport_header = old_transport;
> + return err;
> + }
> +
> return 0;
> }
>
> @@ -198,7 +302,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> const struct virtio_net_hdr *hdr,
> bool little_endian)
> {
> - 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, true);
> }
>
> /* This function must be called after virtio_net_hdr_from_skb(). */
> @@ -293,6 +398,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> unsigned int inner_nh, outer_th, inner_th;
> unsigned int inner_l3min, outer_l3min;
> u8 gso_inner_type, gso_tunnel_type;
> + __be16 inner_proto;
> bool outer_isv6, inner_isv6;
> int ret;
>
> @@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>
> /* 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, false);
> if (ret)
> return ret;
>
> @@ -352,8 +458,8 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> return -EINVAL;
> }
>
> - skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
> - htons(ETH_P_IP));
> + inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
> + skb_set_inner_protocol(skb, inner_proto);
> if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
> if (!tnl_csum_negotiated)
> return -EINVAL;
> @@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> skb->inner_mac_header = inner_nh + skb_headroom(skb);
> skb->transport_header = outer_th + skb_headroom(skb);
> skb->encapsulation = 1;
> - return 0;
> +
> + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> + inner_proto))
> + return 0;
> +
> + if (skb_is_gso(skb))
> + return -EINVAL;
> +
> + ret = skb_checksum_help(skb);
> + return ret ?: 1;
> }
>
> /* Checksum-related fields validation for the driver */
> --
> MST
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
` (4 preceding siblings ...)
2026-10-13 11:05 ` [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
@ 2026-10-13 14:41 ` Willem de Bruijn
2026-10-13 18:41 ` Michael S. Tsirkin
5 siblings, 1 reply; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 14:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> Allowing untrusted hardware to specify csum start/offset for
> CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> outside the actual checksum, and depending on the setup, the networking
> stack could be looking at the packet both before and after the edit, and
> seeing different results.
>
> Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> completing checksum") validated all known instances where this
> causes the most grief, but the real fix is to make sure
> that either only actual checksum fields are ever edited,
> or the checksum is completed before the net stack looks at it,
> so it is always consistent.
+1 on the general approach of performing the checksum offload at
input, unless the input can easily be validated to be one of a few
common protocols. That greatly reduces the risk vs my previous attempt
at trying to drop.
Since we do not know the full extent of grief, I think this too should
go to net and stable. That would mean initially a single patch that
touches as few files as possible. The helper function in
include/linux/ip.h and conversion of existing open coded parsers to
use that can follow up later in net-next.
> This basically does this: checksum at ingress, except in a narrow list
> of specified known good cases.
>
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
>
> Michael S. Tsirkin (4):
> net: add ip4_parse()
> net: use ip4_parse() in receive validation
> net: resolve uncommon partial checksums at ingress
> net: account for ingress-resolved virtio net hdr state
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 11:06 ` Michael S. Tsirkin
@ 2026-10-13 15:03 ` Willem de Bruijn
2026-10-13 18:35 ` Michael S. Tsirkin
2026-10-13 20:15 ` Michael S. Tsirkin
0 siblings, 2 replies; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 15:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 5:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 13, 2026 at 06:58:41AM -0400, Michael S. Tsirkin wrote:
> > A virtio net header can place CHECKSUM_PARTIAL at any in-packet
> > offset. If that state is retained, later checksum completion can
> > modify the packet after protocol processing has made decisions from it.
> >
> > Let's fix that: let's validate csum_start and csum_offset and only keep
> > CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or
> > UDP header, while handling VLAN headers, IPv4 options and IPv6 extension
> > headers. Complete other non-GSO checksums immediately, before any
> > protocol processing runs.
> >
> > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > assumes knowledge of the protocol. We could segment ignoring the
> > checksum then do whatever csum_start and csum_offset say, but that
> > seems like an overkill for something that should happen.
> >
> > UDP tunnel headers get the same handling on the inner header.
> >
> > Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.
> >
> > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: LLM
>
>
> I forgot reported-by tags:
>
> Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Reported-by: Chris Arges <carges@cloudflare.com>
>
>
> > ---
> > include/linux/virtio_net.h | 151 ++++++++++++++++++++++++++++++++-----
> > 1 file changed, 133 insertions(+), 18 deletions(-)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index d6466f96cdd00..b181183118b3a 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -5,7 +5,11 @@
> > #include <linux/if_vlan.h>
> > #include <linux/ip.h>
> > #include <linux/ipv6.h>
> > +#include <linux/netdevice.h>
> > #include <linux/udp.h>
> > +#include <net/gso.h>
> > +#include <net/ip.h>
> > +#include <net/ipv6.h>
> > #include <net/tcp.h>
> > #include <uapi/linux/tcp.h>
> > #include <uapi/linux/virtio_net.h>
> > @@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
> > return 0;
> > }
> >
> > +/* Does checksum metadata match a known TCP or UDP header? */
> > +static inline bool
> > +virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
> > + unsigned int nhoff, __be16 proto)
> > +{
> > + unsigned int thoff;
> > + int ip_proto;
> > +
> > + if (proto == htons(ETH_P_IP)) {
> > + const struct iphdr *iph = ip4_parse(skb, nhoff, &thoff);
> > +
> > + if (!iph || ip_is_fragment(iph))
> > + return false;
> > + ip_proto = iph->protocol;
> > + } else if (proto == htons(ETH_P_IPV6)) {
> > + const struct ipv6hdr *iph;
> > + int flags = 0;
> > +
> > + if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
> > + return false;
> > + iph = (const void *)(skb->data + nhoff);
> > + if (iph->version != 6)
> > + return false;
> > + thoff = nhoff + sizeof(*iph);
> > + ip_proto = iph->nexthdr;
> > + if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_UDP) {
> > + thoff = nhoff;
> > + ip_proto = ipv6_find_hdr(skb, &thoff, -1, NULL,
> > + &flags);
Skip this rare case of extension headers?
> > + if (ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > + return false;
> > + }
> > + } else {
> > + return false;
> > + }
> > +
> > 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,
> > + bool need_csum_check)
> > {
> > - unsigned int nh_min_len = sizeof(struct iphdr);
> > + typeof(skb->transport_header) old_transport = skb->transport_header;
> > unsigned int gso_type = 0;
> > unsigned int thlen = 0;
> > unsigned int p_off = 0;
> > unsigned int ip_proto;
> > + bool csum_partial_ok = true;
> >
> > if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> > @@ -69,7 +169,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > gso_type = SKB_GSO_TCPV6;
> > ip_proto = IPPROTO_TCP;
> > thlen = sizeof(struct tcphdr);
> > - nh_min_len = sizeof(struct ipv6hdr);
> > break;
> > case VIRTIO_NET_HDR_GSO_UDP:
> > gso_type = SKB_GSO_UDP;
> > @@ -104,11 +203,11 @@ 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)
> > - return -EINVAL;
> > + if (need_csum_check)
> > + csum_partial_ok =
> > + virtio_net_csum_partial_ok(skb, hdr_gso_type);
> >
> > - nh_min_len = skb_transport_offset(skb);
> > - p_off = nh_min_len + thlen;
> > + p_off = skb_transport_offset(skb) + thlen;
> > if (!pskb_may_pull(skb, p_off))
> > return -EINVAL;
> > } else if (gso_type) {
> > @@ -158,8 +257,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > case SKB_GSO_UDP_L4:
> > if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> > return -EINVAL;
> > - if (skb->csum_offset != offsetof(struct udphdr, check))
> > - return -EINVAL;
> > if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
> > return -EINVAL;
> > if (gso_type != SKB_GSO_UDP_L4)
> > @@ -167,10 +264,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > break;
> > case SKB_GSO_TCPV4:
> > case SKB_GSO_TCPV6:
> > - if (skb->ip_summed == CHECKSUM_PARTIAL &&
> > - skb->csum_offset != offsetof(struct tcphdr, check))
> > - return -EINVAL;
> > -
> > BUILD_BUG_ON(TCP_MIN_GSO_SIZE * GSO_MAX_SEGS < GSO_MAX_SIZE);
> > gso_size = max(gso_size, TCP_MIN_GSO_SIZE);
> > break;
> > @@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > }
> > }
> >
> > + if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
> > + int err;
> > +
> > + if (skb_is_gso(skb))
> > + return -EINVAL;
> > +
> > + err = skb_checksum_help(skb);
Instead of this variable csum_partial_ok and this branch, could
virtio_net_csum_partial_ok just call skb_checksum_help directly and
drop VIRTIO_NET_HDR_F_NEEDS_CSUM?
> > + skb->transport_header = old_transport;
> > + return err;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -198,7 +302,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > const struct virtio_net_hdr *hdr,
> > bool little_endian)
> > {
> > - 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, true);
Would be great if we can avoid the extra arg and having to update the callers.
> > }
> >
> > /* This function must be called after virtio_net_hdr_from_skb(). */
> > @@ -293,6 +398,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > unsigned int inner_nh, outer_th, inner_th;
> > unsigned int inner_l3min, outer_l3min;
> > u8 gso_inner_type, gso_tunnel_type;
> > + __be16 inner_proto;
> > bool outer_isv6, inner_isv6;
> > int ret;
> >
> > @@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> >
> > /* 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, false);
> > if (ret)
> > return ret;
> >
> > @@ -352,8 +458,8 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > return -EINVAL;
> > }
> >
> > - skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
> > - htons(ETH_P_IP));
> > + inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
> > + skb_set_inner_protocol(skb, inner_proto);
Can drop
> > if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
> > if (!tnl_csum_negotiated)
> > return -EINVAL;
> > @@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > skb->inner_mac_header = inner_nh + skb_headroom(skb);
> > skb->transport_header = outer_th + skb_headroom(skb);
> > skb->encapsulation = 1;
> > - return 0;
> > +
> > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > + inner_proto))
> > + return 0;
> > +
> > + if (skb_is_gso(skb))
> > + return -EINVAL;
This is a risk the commit message will have to call out.
I don't see a simple way around it. We discussed segmenting here too,
but that requires intrusive changes to all callers too.
Alternatively,
1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
thus verify that the TSO/USO packet is valid. Still drop otherwise.
This also does not add overhead, because it would be called later
anyway.
2. a failsafe off-by-default sysctl + static_branch that skips the
drop when set. Messy, but so is the situation we're dealing with.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state
2026-10-13 10:58 ` [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
@ 2026-10-13 17:45 ` Björn Töpel
2026-10-13 18:12 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
1 sibling, 1 reply; 27+ messages in thread
From: Björn Töpel @ 2026-10-13 17:45 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Willem de Bruijn, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell, Simon Horman
"Michael S. Tsirkin" <mst@redhat.com> writes:
> Ingress resolution of uncommon virtio net hdr checksum metadata
> (software checksum completion at ingress) adds performance overhead but
> is currently invisible: no counter tracks it, so a hypothetical workload
> relying on uncommon metadata cannot be distinguished from none at all,
> and packets quietly pay the host-CPU checksum overhead instead of offload.
>
> Have virtio_net_hdr_to_skb() return 1 when it completes an uncommon
> checksum in software, and flip every converter call site to test for
> negative errno. Have tun count these events per device as
> rx_csum_sw and expose the counter via ethtool -S.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: LLM
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
>
> ---
> drivers/net/tap.c | 4 ++--
> drivers/net/tun.c | 44 ++++++++++++++++++++++++++++++++++++--
> drivers/net/virtio_net.c | 2 +-
> include/linux/virtio_net.h | 12 +++++++++--
> net/packet/af_packet.c | 4 ++--
> 5 files changed, 57 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index ff67d99deb39e..73e8833944463 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -649,7 +649,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
>
> if (vnet_hdr_len) {
> err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr);
> - if (err) {
> + if (err < 0) {
> rcu_read_unlock();
> drop_reason = SKB_DROP_REASON_DEV_HDR;
> goto err_kfree;
> @@ -1084,7 +1084,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
>
> if (vnet_hdr_len) {
> err = tun_vnet_hdr_to_skb(q->flags, skb, gso);
> - if (err) {
> + if (err < 0) {
> rcu_read_unlock();
> goto err_kfree;
> }
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 242899f7fd071..138ca8987dfc6 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -210,6 +210,7 @@ struct tun_struct {
> u32 flow_count;
> u32 rx_batched;
> atomic_long_t rx_frame_errors;
> + atomic_long_t rx_csum_sw;
> struct bpf_prog __rcu *xdp_prog;
> struct tun_prog __rcu *steering_prog;
> struct tun_prog __rcu *filter_prog;
> @@ -1935,11 +1936,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> break;
> }
>
> - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> + err = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr);
> + if (err < 0) {
> atomic_long_inc(&tun->rx_frame_errors);
> err = -EINVAL;
> goto free_skb;
> }
> + if (err > 0)
> + atomic_long_inc(&tun->rx_csum_sw);
> + /* The status is consumed: keep err clear so later exit paths
> + * (XDP drop via unlock_frags) do not return it to write() as
> + * a short count.
> + */
> + err = 0;
>
> if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> skb->protocol = eth_type_trans(skb, tun->dev);
> @@ -2605,12 +2614,15 @@ static int tun_xdp_one(struct tun_struct *tun,
> tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> skb->dev = tun->dev;
> skb_set_network_header(skb, ETH_HLEN);
> - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
> + ret = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr);
> + if (ret < 0) {
> atomic_long_inc(&tun->rx_frame_errors);
> kfree_skb(skb);
> ret = -EINVAL;
> goto out;
> }
> + if (ret > 0)
> + atomic_long_inc(&tun->rx_csum_sw);
>
> skb->protocol = eth_type_trans(skb, tun->dev);
> skb_reset_network_header(skb);
> @@ -3739,7 +3751,35 @@ static void tun_get_channels(struct net_device *dev,
> channels->max_combined = tun->flags & IFF_MULTI_QUEUE ? MAX_TAP_QUEUES : 1;
> }
>
> +#define TUN_STATS_LEN 1
> +static const char tun_ethtool_strings[TUN_STATS_LEN][ETH_GSTRING_LEN] = {
> + "rx_csum_sw",
> +};
> +
> +static int tun_get_sset_count(struct net_device *dev, int sset)
> +{
> + return sset == ETH_SS_STATS ? TUN_STATS_LEN : -EOPNOTSUPP;
> +}
> +
> +static void tun_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> +{
> + if (stringset == ETH_SS_STATS)
> + memcpy(data, tun_ethtool_strings,
> + sizeof(tun_ethtool_strings));
> +}
> +
> +static void tun_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> +
> + data[0] = atomic_long_read(&tun->rx_csum_sw);
> +}
> +
> static const struct ethtool_ops tun_ethtool_ops = {
> + .get_sset_count = tun_get_sset_count,
> + .get_strings = tun_get_strings,
> + .get_ethtool_stats = tun_get_ethtool_stats,
> .supported_coalesce_params = ETHTOOL_COALESCE_RX_MAX_FRAMES,
> .get_drvinfo = tun_get_drvinfo,
> .get_msglevel = tun_get_msglevel,
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index daab43ac92ce4..2e6fd81df9c41 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2519,7 +2519,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
> skb_set_network_header(skb, ETH_HLEN);
> if (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)) < 0) {
> 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 b181183118b3a..d8447db58ac83 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -14,6 +14,14 @@
> #include <uapi/linux/tcp.h>
> #include <uapi/linux/virtio_net.h>
>
> +/*
> + * virtio_net_hdr_to_skb() and virtio_net_hdr_tnl_to_skb() return 0 on
> + * success and a negative errno on failure. On success they may also
> + * return 1, indicating that an uncommon checksum was completed in
> + * software at ingress rather than deferred to the egress path;
> + * drivers can use this to account for such packets.
> + */
> +
> static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
> {
> switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> @@ -292,7 +300,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>
> err = skb_checksum_help(skb);
> skb->transport_header = old_transport;
> - return err;
> + return err ?: 1;
> }
>
> return 0;
> @@ -441,7 +449,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> /* Let the basic parsing deal with plain GSO features. */
> ret = __virtio_net_hdr_to_skb(skb, hdr, true,
> hdr->gso_type & ~gso_tunnel_type, false);
> - if (ret)
> + if (ret < 0)
> return ret;
>
> /* In case of USO, the inner protocol is still unknown and
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 52ae3d875e971..e6aec9ecfa994 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2913,7 +2913,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 (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()) < 0) {
nit: I don't know if we care, but this sort of breaks bisectability.
Previous patch does return ret ?: 1, but it's this patch that does the
if-statement fixup. (Same for tun/tap... ^^^)
Björn
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state
2026-10-13 17:45 ` Björn Töpel
@ 2026-10-13 18:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 18:12 UTC (permalink / raw)
To: Björn Töpel
Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez,
Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, virtualization,
Max Krasnyansky, Rusty Russell, Simon Horman
On Tue, Oct 13, 2026 at 07:45:17PM +0200, Björn Töpel wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> > Ingress resolution of uncommon virtio net hdr checksum metadata
> > (software checksum completion at ingress) adds performance overhead but
> > is currently invisible: no counter tracks it, so a hypothetical workload
> > relying on uncommon metadata cannot be distinguished from none at all,
> > and packets quietly pay the host-CPU checksum overhead instead of offload.
> >
> > Have virtio_net_hdr_to_skb() return 1 when it completes an uncommon
> > checksum in software, and flip every converter call site to test for
> > negative errno. Have tun count these events per device as
> > rx_csum_sw and expose the counter via ethtool -S.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: LLM
> > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> >
> > ---
> > drivers/net/tap.c | 4 ++--
> > drivers/net/tun.c | 44 ++++++++++++++++++++++++++++++++++++--
> > drivers/net/virtio_net.c | 2 +-
> > include/linux/virtio_net.h | 12 +++++++++--
> > net/packet/af_packet.c | 4 ++--
> > 5 files changed, 57 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> > index ff67d99deb39e..73e8833944463 100644
> > --- a/drivers/net/tap.c
> > +++ b/drivers/net/tap.c
> > @@ -649,7 +649,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
> >
> > if (vnet_hdr_len) {
> > err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr);
> > - if (err) {
> > + if (err < 0) {
> > rcu_read_unlock();
> > drop_reason = SKB_DROP_REASON_DEV_HDR;
> > goto err_kfree;
> > @@ -1084,7 +1084,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
> >
> > if (vnet_hdr_len) {
> > err = tun_vnet_hdr_to_skb(q->flags, skb, gso);
> > - if (err) {
> > + if (err < 0) {
> > rcu_read_unlock();
> > goto err_kfree;
> > }
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 242899f7fd071..138ca8987dfc6 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -210,6 +210,7 @@ struct tun_struct {
> > u32 flow_count;
> > u32 rx_batched;
> > atomic_long_t rx_frame_errors;
> > + atomic_long_t rx_csum_sw;
> > struct bpf_prog __rcu *xdp_prog;
> > struct tun_prog __rcu *steering_prog;
> > struct tun_prog __rcu *filter_prog;
> > @@ -1935,11 +1936,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > break;
> > }
> >
> > - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> > + err = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr);
> > + if (err < 0) {
> > atomic_long_inc(&tun->rx_frame_errors);
> > err = -EINVAL;
> > goto free_skb;
> > }
> > + if (err > 0)
> > + atomic_long_inc(&tun->rx_csum_sw);
> > + /* The status is consumed: keep err clear so later exit paths
> > + * (XDP drop via unlock_frags) do not return it to write() as
> > + * a short count.
> > + */
> > + err = 0;
> >
> > if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> > skb->protocol = eth_type_trans(skb, tun->dev);
> > @@ -2605,12 +2614,15 @@ static int tun_xdp_one(struct tun_struct *tun,
> > tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
> > skb->dev = tun->dev;
> > skb_set_network_header(skb, ETH_HLEN);
> > - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
> > + ret = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr);
> > + if (ret < 0) {
> > atomic_long_inc(&tun->rx_frame_errors);
> > kfree_skb(skb);
> > ret = -EINVAL;
> > goto out;
> > }
> > + if (ret > 0)
> > + atomic_long_inc(&tun->rx_csum_sw);
> >
> > skb->protocol = eth_type_trans(skb, tun->dev);
> > skb_reset_network_header(skb);
> > @@ -3739,7 +3751,35 @@ static void tun_get_channels(struct net_device *dev,
> > channels->max_combined = tun->flags & IFF_MULTI_QUEUE ? MAX_TAP_QUEUES : 1;
> > }
> >
> > +#define TUN_STATS_LEN 1
> > +static const char tun_ethtool_strings[TUN_STATS_LEN][ETH_GSTRING_LEN] = {
> > + "rx_csum_sw",
> > +};
> > +
> > +static int tun_get_sset_count(struct net_device *dev, int sset)
> > +{
> > + return sset == ETH_SS_STATS ? TUN_STATS_LEN : -EOPNOTSUPP;
> > +}
> > +
> > +static void tun_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> > +{
> > + if (stringset == ETH_SS_STATS)
> > + memcpy(data, tun_ethtool_strings,
> > + sizeof(tun_ethtool_strings));
> > +}
> > +
> > +static void tun_get_ethtool_stats(struct net_device *dev,
> > + struct ethtool_stats *stats, u64 *data)
> > +{
> > + struct tun_struct *tun = netdev_priv(dev);
> > +
> > + data[0] = atomic_long_read(&tun->rx_csum_sw);
> > +}
> > +
> > static const struct ethtool_ops tun_ethtool_ops = {
> > + .get_sset_count = tun_get_sset_count,
> > + .get_strings = tun_get_strings,
> > + .get_ethtool_stats = tun_get_ethtool_stats,
> > .supported_coalesce_params = ETHTOOL_COALESCE_RX_MAX_FRAMES,
> > .get_drvinfo = tun_get_drvinfo,
> > .get_msglevel = tun_get_msglevel,
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index daab43ac92ce4..2e6fd81df9c41 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2519,7 +2519,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
> > skb_set_network_header(skb, ETH_HLEN);
> > if (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)) < 0) {
> > 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 b181183118b3a..d8447db58ac83 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -14,6 +14,14 @@
> > #include <uapi/linux/tcp.h>
> > #include <uapi/linux/virtio_net.h>
> >
> > +/*
> > + * virtio_net_hdr_to_skb() and virtio_net_hdr_tnl_to_skb() return 0 on
> > + * success and a negative errno on failure. On success they may also
> > + * return 1, indicating that an uncommon checksum was completed in
> > + * software at ingress rather than deferred to the egress path;
> > + * drivers can use this to account for such packets.
> > + */
> > +
> > static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
> > {
> > switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> > @@ -292,7 +300,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> >
> > err = skb_checksum_help(skb);
> > skb->transport_header = old_transport;
> > - return err;
> > + return err ?: 1;
> > }
> >
> > return 0;
> > @@ -441,7 +449,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > /* Let the basic parsing deal with plain GSO features. */
> > ret = __virtio_net_hdr_to_skb(skb, hdr, true,
> > hdr->gso_type & ~gso_tunnel_type, false);
> > - if (ret)
> > + if (ret < 0)
> > return ret;
> >
> > /* In case of USO, the inner protocol is still unknown and
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index 52ae3d875e971..e6aec9ecfa994 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -2913,7 +2913,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 (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()) < 0) {
>
> nit: I don't know if we care, but this sort of breaks bisectability.
> Previous patch does return ret ?: 1, but it's this patch that does the
> if-statement fixup. (Same for tun/tap... ^^^)
>
Oh ugh. You are right, I kept rebasing to tweak things and moved it
by chance. I'll fix it up in v2.
> Björn
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 15:03 ` Willem de Bruijn
@ 2026-10-13 18:35 ` Michael S. Tsirkin
2026-10-13 20:32 ` Willem de Bruijn
2026-10-13 20:15 ` Michael S. Tsirkin
1 sibling, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 18:35 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 09:03:04AM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 5:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 06:58:41AM -0400, Michael S. Tsirkin wrote:
> > > A virtio net header can place CHECKSUM_PARTIAL at any in-packet
> > > offset. If that state is retained, later checksum completion can
> > > modify the packet after protocol processing has made decisions from it.
> > >
> > > Let's fix that: let's validate csum_start and csum_offset and only keep
> > > CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or
> > > UDP header, while handling VLAN headers, IPv4 options and IPv6 extension
> > > headers. Complete other non-GSO checksums immediately, before any
> > > protocol processing runs.
> > >
> > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > assumes knowledge of the protocol. We could segment ignoring the
> > > checksum then do whatever csum_start and csum_offset say, but that
> > > seems like an overkill for something that should happen.
> > >
> > > UDP tunnel headers get the same handling on the inner header.
> > >
> > > Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.
> > >
> > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Assisted-by: LLM
> >
> >
> > I forgot reported-by tags:
> >
> > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> > Reported-by: Chris Arges <carges@cloudflare.com>
> >
> >
> > > ---
> > > include/linux/virtio_net.h | 151 ++++++++++++++++++++++++++++++++-----
> > > 1 file changed, 133 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index d6466f96cdd00..b181183118b3a 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -5,7 +5,11 @@
> > > #include <linux/if_vlan.h>
> > > #include <linux/ip.h>
> > > #include <linux/ipv6.h>
> > > +#include <linux/netdevice.h>
> > > #include <linux/udp.h>
> > > +#include <net/gso.h>
> > > +#include <net/ip.h>
> > > +#include <net/ipv6.h>
> > > #include <net/tcp.h>
> > > #include <uapi/linux/tcp.h>
> > > #include <uapi/linux/virtio_net.h>
> > > @@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
> > > return 0;
> > > }
> > >
> > > +/* Does checksum metadata match a known TCP or UDP header? */
> > > +static inline bool
> > > +virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
> > > + unsigned int nhoff, __be16 proto)
> > > +{
> > > + unsigned int thoff;
> > > + int ip_proto;
> > > +
> > > + if (proto == htons(ETH_P_IP)) {
> > > + const struct iphdr *iph = ip4_parse(skb, nhoff, &thoff);
> > > +
> > > + if (!iph || ip_is_fragment(iph))
> > > + return false;
> > > + ip_proto = iph->protocol;
> > > + } else if (proto == htons(ETH_P_IPV6)) {
> > > + const struct ipv6hdr *iph;
> > > + int flags = 0;
> > > +
> > > + if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
> > > + return false;
> > > + iph = (const void *)(skb->data + nhoff);
> > > + if (iph->version != 6)
> > > + return false;
> > > + thoff = nhoff + sizeof(*iph);
> > > + ip_proto = iph->nexthdr;
> > > + if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_UDP) {
> > > + thoff = nhoff;
> > > + ip_proto = ipv6_find_hdr(skb, &thoff, -1, NULL,
> > > + &flags);
>
> Skip this rare case of extension headers?
>
> > > + if (ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > > + return false;
> > > + }
> > > + } else {
> > > + return false;
> > > + }
> > > +
>
> > > 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,
> > > + bool need_csum_check)
> > > {
> > > - unsigned int nh_min_len = sizeof(struct iphdr);
> > > + typeof(skb->transport_header) old_transport = skb->transport_header;
> > > unsigned int gso_type = 0;
> > > unsigned int thlen = 0;
> > > unsigned int p_off = 0;
> > > unsigned int ip_proto;
> > > + bool csum_partial_ok = true;
> > >
> > > if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > > switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> > > @@ -69,7 +169,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > gso_type = SKB_GSO_TCPV6;
> > > ip_proto = IPPROTO_TCP;
> > > thlen = sizeof(struct tcphdr);
> > > - nh_min_len = sizeof(struct ipv6hdr);
> > > break;
> > > case VIRTIO_NET_HDR_GSO_UDP:
> > > gso_type = SKB_GSO_UDP;
> > > @@ -104,11 +203,11 @@ 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)
> > > - return -EINVAL;
> > > + if (need_csum_check)
> > > + csum_partial_ok =
> > > + virtio_net_csum_partial_ok(skb, hdr_gso_type);
> > >
> > > - nh_min_len = skb_transport_offset(skb);
> > > - p_off = nh_min_len + thlen;
> > > + p_off = skb_transport_offset(skb) + thlen;
> > > if (!pskb_may_pull(skb, p_off))
> > > return -EINVAL;
> > > } else if (gso_type) {
> > > @@ -158,8 +257,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > case SKB_GSO_UDP_L4:
> > > if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> > > return -EINVAL;
> > > - if (skb->csum_offset != offsetof(struct udphdr, check))
> > > - return -EINVAL;
> > > if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
> > > return -EINVAL;
> > > if (gso_type != SKB_GSO_UDP_L4)
> > > @@ -167,10 +264,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > break;
> > > case SKB_GSO_TCPV4:
> > > case SKB_GSO_TCPV6:
> > > - if (skb->ip_summed == CHECKSUM_PARTIAL &&
> > > - skb->csum_offset != offsetof(struct tcphdr, check))
> > > - return -EINVAL;
> > > -
> > > BUILD_BUG_ON(TCP_MIN_GSO_SIZE * GSO_MAX_SEGS < GSO_MAX_SIZE);
> > > gso_size = max(gso_size, TCP_MIN_GSO_SIZE);
> > > break;
> > > @@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > }
> > > }
> > >
> > > + if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
> > > + int err;
> > > +
> > > + if (skb_is_gso(skb))
> > > + return -EINVAL;
> > > +
> > > + err = skb_checksum_help(skb);
>
> Instead of this variable csum_partial_ok and this branch, could
> virtio_net_csum_partial_ok just call skb_checksum_help directly and
> drop VIRTIO_NET_HDR_F_NEEDS_CSUM?
>
> > > + skb->transport_header = old_transport;
> > > + return err;
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -198,7 +302,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > const struct virtio_net_hdr *hdr,
> > > bool little_endian)
> > > {
> > > - 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, true);
>
> Would be great if we can avoid the extra arg and having to update the callers.
>
> > > }
> > >
> > > /* This function must be called after virtio_net_hdr_from_skb(). */
> > > @@ -293,6 +398,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > unsigned int inner_nh, outer_th, inner_th;
> > > unsigned int inner_l3min, outer_l3min;
> > > u8 gso_inner_type, gso_tunnel_type;
> > > + __be16 inner_proto;
> > > bool outer_isv6, inner_isv6;
> > > int ret;
> > >
> > > @@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > >
> > > /* 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, false);
> > > if (ret)
> > > return ret;
> > >
> > > @@ -352,8 +458,8 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > return -EINVAL;
> > > }
> > >
> > > - skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
> > > - htons(ETH_P_IP));
> > > + inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
> > > + skb_set_inner_protocol(skb, inner_proto);
>
> Can drop
>
> > > if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
> > > if (!tnl_csum_negotiated)
> > > return -EINVAL;
> > > @@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > skb->inner_mac_header = inner_nh + skb_headroom(skb);
> > > skb->transport_header = outer_th + skb_headroom(skb);
> > > skb->encapsulation = 1;
> > > - return 0;
> > > +
> > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > + inner_proto))
> > > + return 0;
> > > +
> > > + if (skb_is_gso(skb))
> > > + return -EINVAL;
>
> This is a risk the commit message will have to call out.
I tried with this:
> > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > assumes knowledge of the protocol. We could segment ignoring the
> > > checksum then do whatever csum_start and csum_offset say, but that
> > > seems like an overkill for something that should happen.
I guess I can add a more detailed explanation about what the risk is?
> I don't see a simple way around it. We discussed segmenting here too,
> but that requires intrusive changes to all callers too.
> Alternatively,
>
> 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> thus verify that the TSO/USO packet is valid. Still drop otherwise.
> This also does not add overhead, because it would be called later
> anyway.
>
> 2. a failsafe off-by-default sysctl + static_branch that skips the
> drop when set. Messy, but so is the situation we're dealing with.
Tack this on top or do you feel it's needed straight away.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 14:41 ` Willem de Bruijn
@ 2026-10-13 18:41 ` Michael S. Tsirkin
2026-10-13 20:29 ` Willem de Bruijn
0 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 18:41 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 08:41:19AM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > Allowing untrusted hardware to specify csum start/offset for
> > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > outside the actual checksum, and depending on the setup, the networking
> > stack could be looking at the packet both before and after the edit, and
> > seeing different results.
> >
> > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > completing checksum") validated all known instances where this
> > causes the most grief, but the real fix is to make sure
> > that either only actual checksum fields are ever edited,
> > or the checksum is completed before the net stack looks at it,
> > so it is always consistent.
>
> +1 on the general approach of performing the checksum offload at
> input, unless the input can easily be validated to be one of a few
> common protocols. That greatly reduces the risk vs my previous attempt
> at trying to drop.
>
> Since we do not know the full extent of grief, I think this too should
> go to net and stable.
I think it needs a bit more testing to be frank, I do not see it
wait months out of the tree. So if this lands in next before the
merge window, we are good I think. If we miss that I will target net.
> That would mean initially a single patch that
> touches as few files as possible. The helper function in
> include/linux/ip.h and conversion of existing open coded parsers to
> use that can follow up later in net-next.
I don't see the value to be frank.
What moving code between files accomplishes, I donnu.
Squashing things surely makes review harder?
Does your backporting system really struggle with trivial
dependencies like this?
I can drop patch 2 and will if I am targeting net.
>
> > This basically does this: checksum at ingress, except in a narrow list
> > of specified known good cases.
> >
> > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> >
> > Michael S. Tsirkin (4):
> > net: add ip4_parse()
> > net: use ip4_parse() in receive validation
> > net: resolve uncommon partial checksums at ingress
> > net: account for ingress-resolved virtio net hdr state
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 11:05 ` [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
@ 2026-10-13 18:51 ` Chris Arges
0 siblings, 0 replies; 27+ messages in thread
From: Chris Arges @ 2026-10-13 18:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Willem de Bruijn, Jason Wang, Eugenio Pérez,
Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, virtualization,
Max Krasnyansky, Paulos Yibelo
On 2026-10-13 07:05:34, Michael S. Tsirkin wrote:
> On Tue, Oct 13, 2026 at 06:58:28AM -0400, Michael S. Tsirkin wrote:
> > Allowing untrusted hardware to specify csum start/offset for
> > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > outside the actual checksum, and depending on the setup, the networking
> > stack could be looking at the packet both before and after the edit, and
> > seeing different results.
> >
> > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > completing checksum") validated all known instances where this
> > causes the most grief, but the real fix is to make sure
> > that either only actual checksum fields are ever edited,
> > or the checksum is completed before the net stack looks at it,
> > so it is always consistent.
> >
> > This basically does this: checksum at ingress, except in a narrow list
> > of specified known good cases.
> >
> > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
>
>
> I forgot.
>
> Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Reported-by: Chris Arges <carges@cloudflare.com>
>
>
I did some testing of this patch-set and it mitigates my cases in VM
testing.
Tested-by: Chris Arges <carges@cloudflare.com>
> > Michael S. Tsirkin (4):
> > net: add ip4_parse()
> > net: use ip4_parse() in receive validation
> > net: resolve uncommon partial checksums at ingress
> > net: account for ingress-resolved virtio net hdr state
> >
> > drivers/net/tap.c | 4 +-
> > drivers/net/tun.c | 44 +++++++-
> > drivers/net/virtio_net.c | 2 +-
> > include/linux/ip.h | 24 +++++
> > include/linux/virtio_net.h | 161 ++++++++++++++++++++++++----
> > net/bridge/br_netfilter_hooks.c | 15 +--
> > net/ipv4/ip_input.c | 13 +--
> > net/ipv4/netfilter/nf_reject_ipv4.c | 13 +--
> > net/packet/af_packet.c | 4 +-
> > 9 files changed, 228 insertions(+), 52 deletions(-)
> >
> > --
> > MST
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 15:03 ` Willem de Bruijn
2026-10-13 18:35 ` Michael S. Tsirkin
@ 2026-10-13 20:15 ` Michael S. Tsirkin
1 sibling, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 20:15 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 09:03:04AM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 5:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 06:58:41AM -0400, Michael S. Tsirkin wrote:
> > > A virtio net header can place CHECKSUM_PARTIAL at any in-packet
> > > offset. If that state is retained, later checksum completion can
> > > modify the packet after protocol processing has made decisions from it.
> > >
> > > Let's fix that: let's validate csum_start and csum_offset and only keep
> > > CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or
> > > UDP header, while handling VLAN headers, IPv4 options and IPv6 extension
> > > headers. Complete other non-GSO checksums immediately, before any
> > > protocol processing runs.
> > >
> > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > assumes knowledge of the protocol. We could segment ignoring the
> > > checksum then do whatever csum_start and csum_offset say, but that
> > > seems like an overkill for something that should happen.
> > >
> > > UDP tunnel headers get the same handling on the inner header.
> > >
> > > Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.
> > >
> > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Assisted-by: LLM
> >
> >
> > I forgot reported-by tags:
> >
> > Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> > Reported-by: Chris Arges <carges@cloudflare.com>
> >
> >
> > > ---
> > > include/linux/virtio_net.h | 151 ++++++++++++++++++++++++++++++++-----
> > > 1 file changed, 133 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index d6466f96cdd00..b181183118b3a 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -5,7 +5,11 @@
> > > #include <linux/if_vlan.h>
> > > #include <linux/ip.h>
> > > #include <linux/ipv6.h>
> > > +#include <linux/netdevice.h>
> > > #include <linux/udp.h>
> > > +#include <net/gso.h>
> > > +#include <net/ip.h>
> > > +#include <net/ipv6.h>
> > > #include <net/tcp.h>
> > > #include <uapi/linux/tcp.h>
> > > #include <uapi/linux/virtio_net.h>
> > > @@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
> > > return 0;
> > > }
> > >
> > > +/* Does checksum metadata match a known TCP or UDP header? */
> > > +static inline bool
> > > +virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
> > > + unsigned int nhoff, __be16 proto)
> > > +{
> > > + unsigned int thoff;
> > > + int ip_proto;
> > > +
> > > + if (proto == htons(ETH_P_IP)) {
> > > + const struct iphdr *iph = ip4_parse(skb, nhoff, &thoff);
> > > +
> > > + if (!iph || ip_is_fragment(iph))
> > > + return false;
> > > + ip_proto = iph->protocol;
> > > + } else if (proto == htons(ETH_P_IPV6)) {
> > > + const struct ipv6hdr *iph;
> > > + int flags = 0;
> > > +
> > > + if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
> > > + return false;
> > > + iph = (const void *)(skb->data + nhoff);
> > > + if (iph->version != 6)
> > > + return false;
> > > + thoff = nhoff + sizeof(*iph);
> > > + ip_proto = iph->nexthdr;
> > > + if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_UDP) {
> > > + thoff = nhoff;
> > > + ip_proto = ipv6_find_hdr(skb, &thoff, -1, NULL,
> > > + &flags);
>
> Skip this rare case of extension headers?
but then it will be dropped if it's gso. so we can't.
> > > + if (ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > > + return false;
> > > + }
> > > + } else {
> > > + return false;
> > > + }
> > > +
>
> > > 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,
> > > + bool need_csum_check)
> > > {
> > > - unsigned int nh_min_len = sizeof(struct iphdr);
> > > + typeof(skb->transport_header) old_transport = skb->transport_header;
> > > unsigned int gso_type = 0;
> > > unsigned int thlen = 0;
> > > unsigned int p_off = 0;
> > > unsigned int ip_proto;
> > > + bool csum_partial_ok = true;
> > >
> > > if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > > switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> > > @@ -69,7 +169,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > gso_type = SKB_GSO_TCPV6;
> > > ip_proto = IPPROTO_TCP;
> > > thlen = sizeof(struct tcphdr);
> > > - nh_min_len = sizeof(struct ipv6hdr);
> > > break;
> > > case VIRTIO_NET_HDR_GSO_UDP:
> > > gso_type = SKB_GSO_UDP;
> > > @@ -104,11 +203,11 @@ 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)
> > > - return -EINVAL;
> > > + if (need_csum_check)
> > > + csum_partial_ok =
> > > + virtio_net_csum_partial_ok(skb, hdr_gso_type);
> > >
> > > - nh_min_len = skb_transport_offset(skb);
> > > - p_off = nh_min_len + thlen;
> > > + p_off = skb_transport_offset(skb) + thlen;
> > > if (!pskb_may_pull(skb, p_off))
> > > return -EINVAL;
> > > } else if (gso_type) {
> > > @@ -158,8 +257,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > case SKB_GSO_UDP_L4:
> > > if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> > > return -EINVAL;
> > > - if (skb->csum_offset != offsetof(struct udphdr, check))
> > > - return -EINVAL;
> > > if (skb->len - p_off > gso_size * UDP_MAX_SEGMENTS)
> > > return -EINVAL;
> > > if (gso_type != SKB_GSO_UDP_L4)
> > > @@ -167,10 +264,6 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > break;
> > > case SKB_GSO_TCPV4:
> > > case SKB_GSO_TCPV6:
> > > - if (skb->ip_summed == CHECKSUM_PARTIAL &&
> > > - skb->csum_offset != offsetof(struct tcphdr, check))
> > > - return -EINVAL;
> > > -
> > > BUILD_BUG_ON(TCP_MIN_GSO_SIZE * GSO_MAX_SEGS < GSO_MAX_SIZE);
> > > gso_size = max(gso_size, TCP_MIN_GSO_SIZE);
> > > break;
> > > @@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > }
> > > }
> > >
> > > + if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
> > > + int err;
> > > +
> > > + if (skb_is_gso(skb))
> > > + return -EINVAL;
> > > +
> > > + err = skb_checksum_help(skb);
>
> Instead of this variable csum_partial_ok and this branch, could
> virtio_net_csum_partial_ok just call skb_checksum_help directly and
> drop VIRTIO_NET_HDR_F_NEEDS_CSUM?
no, hdr is const. and I think I like it that way. we could
copy flags I guess - I'll play with it a bit.
> > > + skb->transport_header = old_transport;
> > > + return err;
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -198,7 +302,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > const struct virtio_net_hdr *hdr,
> > > bool little_endian)
> > > {
> > > - 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, true);
>
> Would be great if we can avoid the extra arg and having to update the callers.
There are 2 callers why is this a concern?
But yes the extra bool is kinda ugly. i'll look into refactoring
this but i do not think the diff will be smaller - likely larger.
> > > }
> > >
> > > /* This function must be called after virtio_net_hdr_from_skb(). */
> > > @@ -293,6 +398,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > unsigned int inner_nh, outer_th, inner_th;
> > > unsigned int inner_l3min, outer_l3min;
> > > u8 gso_inner_type, gso_tunnel_type;
> > > + __be16 inner_proto;
> > > bool outer_isv6, inner_isv6;
> > > int ret;
> > >
> > > @@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > >
> > > /* 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, false);
> > > if (ret)
> > > return ret;
> > >
> > > @@ -352,8 +458,8 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > return -EINVAL;
> > > }
> > >
> > > - skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
> > > - htons(ETH_P_IP));
> > > + inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
> > > + skb_set_inner_protocol(skb, inner_proto);
>
> Can drop
It is here because inner_proto is used below, seems silly to re-compute
when we have it.
> > > if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
> > > if (!tnl_csum_negotiated)
> > > return -EINVAL;
> > > @@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> > > skb->inner_mac_header = inner_nh + skb_headroom(skb);
> > > skb->transport_header = outer_th + skb_headroom(skb);
> > > skb->encapsulation = 1;
> > > - return 0;
> > > +
> > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > + inner_proto))
> > > + return 0;
> > > +
> > > + if (skb_is_gso(skb))
> > > + return -EINVAL;
>
> This is a risk the commit message will have to call out.
I tried but good point, not enough. Will do.
> I don't see a simple way around it. We discussed segmenting here too,
> but that requires intrusive changes to all callers too.
>
> Alternatively,
>
> 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> thus verify that the TSO/USO packet is valid. Still drop otherwise.
> This also does not add overhead, because it would be called later
> anyway.
Yes, I implemented this originally. It's an even more messy code but the
big issue is that it's not clear it is worth it. So we get gso with
a bad checksum. Currently it goes out on the wire corrupted. Ideally
we would do exactly that, but that would mean much more work that
SKB_GSO_DODGY play. Conversely we could just ignore the
checksum hints. Then the packet would go on wire because we
"fixed" it but it just has a chance to cause even more problems.
So... shrug. I opted for the simpler way.
So... shrug. I opted for the simpler way. If a real workload shows
up, the counters in the last patch will tell us and we can revisit.
I will add this to the cover letter I feel, not commit log material,
right?
> 2. a failsafe off-by-default sysctl + static_branch that skips the
> drop when set. Messy, but so is the situation we're dealing with.
good idea, I'll add that.
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 18:41 ` Michael S. Tsirkin
@ 2026-10-13 20:29 ` Willem de Bruijn
2026-10-13 20:36 ` Michael S. Tsirkin
0 siblings, 1 reply; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 20:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 12:41 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 13, 2026 at 08:41:19AM -0600, Willem de Bruijn wrote:
> > On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > Allowing untrusted hardware to specify csum start/offset for
> > > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > > outside the actual checksum, and depending on the setup, the networking
> > > stack could be looking at the packet both before and after the edit, and
> > > seeing different results.
> > >
> > > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > > completing checksum") validated all known instances where this
> > > causes the most grief, but the real fix is to make sure
> > > that either only actual checksum fields are ever edited,
> > > or the checksum is completed before the net stack looks at it,
> > > so it is always consistent.
> >
> > +1 on the general approach of performing the checksum offload at
> > input, unless the input can easily be validated to be one of a few
> > common protocols. That greatly reduces the risk vs my previous attempt
> > at trying to drop.
> >
> > Since we do not know the full extent of grief, I think this too should
> > go to net and stable.
>
> I think it needs a bit more testing to be frank, I do not see it
> wait months out of the tree. So if this lands in next before the
> merge window, we are good I think. If we miss that I will target net.
>
>
> > That would mean initially a single patch that
> > touches as few files as possible. The helper function in
> > include/linux/ip.h and conversion of existing open coded parsers to
> > use that can follow up later in net-next.
>
> I don't see the value to be frank.
> What moving code between files accomplishes, I donnu.
> Squashing things surely makes review harder?
> Does your backporting system really struggle with trivial
> dependencies like this?
>
> I can drop patch 2 and will if I am targeting net.
Good. This is not about "my" backporting, but upstream stable.
Refactor patches do not belong there.
Since patch 3 might have to be backported in the future if a serious
abuse surfaces, even if you now land it in net-next, it is better to
not depend on other patches.
> >
> > > This basically does this: checksum at ingress, except in a narrow list
> > > of specified known good cases.
> > >
> > > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> > >
> > > Michael S. Tsirkin (4):
> > > net: add ip4_parse()
> > > net: use ip4_parse() in receive validation
> > > net: resolve uncommon partial checksums at ingress
> > > net: account for ingress-resolved virtio net hdr state
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 18:35 ` Michael S. Tsirkin
@ 2026-10-13 20:32 ` Willem de Bruijn
2026-10-13 20:38 ` Michael S. Tsirkin
0 siblings, 1 reply; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 20:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
> > > > +
> > > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > > + inner_proto))
> > > > + return 0;
> > > > +
> > > > + if (skb_is_gso(skb))
> > > > + return -EINVAL;
> >
> > This is a risk the commit message will have to call out.
>
> I tried with this:
>
> > > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > > assumes knowledge of the protocol. We could segment ignoring the
> > > > checksum then do whatever csum_start and csum_offset say, but that
> > > > seems like an overkill for something that should happen.
>
> I guess I can add a more detailed explanation about what the risk is?
Let's just explicitly mention the consequence: non-standard GSO
packets that previous would be sent may now fail.
>
> > I don't see a simple way around it. We discussed segmenting here too,
> > but that requires intrusive changes to all callers too.
> > Alternatively,
> >
> > 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> > thus verify that the TSO/USO packet is valid. Still drop otherwise.
> > This also does not add overhead, because it would be called later
> > anyway.
> >
> > 2. a failsafe off-by-default sysctl + static_branch that skips the
> > drop when set. Messy, but so is the situation we're dealing with.
>
> Tack this on top or do you feel it's needed straight away.
Not urgent. Can even wait until a report appears. But worth taking a
look sooner if either of us gets around to it.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 20:29 ` Willem de Bruijn
@ 2026-10-13 20:36 ` Michael S. Tsirkin
2026-10-13 20:54 ` Willem de Bruijn
0 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 20:36 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 02:29:53PM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 12:41 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 08:41:19AM -0600, Willem de Bruijn wrote:
> > > On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > Allowing untrusted hardware to specify csum start/offset for
> > > > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > > > outside the actual checksum, and depending on the setup, the networking
> > > > stack could be looking at the packet both before and after the edit, and
> > > > seeing different results.
> > > >
> > > > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > > > completing checksum") validated all known instances where this
> > > > causes the most grief, but the real fix is to make sure
> > > > that either only actual checksum fields are ever edited,
> > > > or the checksum is completed before the net stack looks at it,
> > > > so it is always consistent.
> > >
> > > +1 on the general approach of performing the checksum offload at
> > > input, unless the input can easily be validated to be one of a few
> > > common protocols. That greatly reduces the risk vs my previous attempt
> > > at trying to drop.
> > >
> > > Since we do not know the full extent of grief, I think this too should
> > > go to net and stable.
> >
> > I think it needs a bit more testing to be frank, I do not see it
> > wait months out of the tree. So if this lands in next before the
> > merge window, we are good I think. If we miss that I will target net.
> >
> >
> > > That would mean initially a single patch that
> > > touches as few files as possible. The helper function in
> > > include/linux/ip.h and conversion of existing open coded parsers to
> > > use that can follow up later in net-next.
> >
> > I don't see the value to be frank.
> > What moving code between files accomplishes, I donnu.
> > Squashing things surely makes review harder?
> > Does your backporting system really struggle with trivial
> > dependencies like this?
> >
> > I can drop patch 2 and will if I am targeting net.
>
> Good. This is not about "my" backporting, but upstream stable.
> Refactor patches do not belong there.
>
> Since patch 3 might have to be backported in the future if a serious
> abuse surfaces, even if you now land it in net-next, it is better to
> not depend on other patches.
Right. While this is targeting next, I will reorder it last in series so
it's obvious for people it can be dropped.
Thanks a lot for the review!
> > >
> > > > This basically does this: checksum at ingress, except in a narrow list
> > > > of specified known good cases.
> > > >
> > > > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> > > >
> > > > Michael S. Tsirkin (4):
> > > > net: add ip4_parse()
> > > > net: use ip4_parse() in receive validation
> > > > net: resolve uncommon partial checksums at ingress
> > > > net: account for ingress-resolved virtio net hdr state
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 20:32 ` Willem de Bruijn
@ 2026-10-13 20:38 ` Michael S. Tsirkin
2026-10-13 20:45 ` Willem de Bruijn
0 siblings, 1 reply; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 20:38 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 02:32:26PM -0600, Willem de Bruijn wrote:
> > > > > +
> > > > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > > > + inner_proto))
> > > > > + return 0;
> > > > > +
> > > > > + if (skb_is_gso(skb))
> > > > > + return -EINVAL;
> > >
> > > This is a risk the commit message will have to call out.
> >
> > I tried with this:
> >
> > > > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > > > assumes knowledge of the protocol. We could segment ignoring the
> > > > > checksum then do whatever csum_start and csum_offset say, but that
> > > > > seems like an overkill for something that should happen.
> >
> > I guess I can add a more detailed explanation about what the risk is?
>
> Let's just explicitly mention the consequence: non-standard GSO
> packets that previous would be sent may now fail.
>
> >
> > > I don't see a simple way around it. We discussed segmenting here too,
> > > but that requires intrusive changes to all callers too.
> > > Alternatively,
> > >
> > > 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> > > thus verify that the TSO/USO packet is valid. Still drop otherwise.
> > > This also does not add overhead, because it would be called later
> > > anyway.
> > >
> > > 2. a failsafe off-by-default sysctl + static_branch that skips the
> > > drop when set. Messy, but so is the situation we're dealing with.
> >
> > Tack this on top or do you feel it's needed straight away.
>
> Not urgent. Can even wait until a report appears. But worth taking a
> look sooner if either of us gets around to it.
Maybe if people intend all this for stable, I'll feel safer with this in
there.
Just to make sure, the branch would just short-circuit all checks, yes?
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 20:38 ` Michael S. Tsirkin
@ 2026-10-13 20:45 ` Willem de Bruijn
2026-10-13 20:51 ` Michael S. Tsirkin
0 siblings, 1 reply; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 20:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 2:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 13, 2026 at 02:32:26PM -0600, Willem de Bruijn wrote:
> > > > > > +
> > > > > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > > > > + inner_proto))
> > > > > > + return 0;
> > > > > > +
> > > > > > + if (skb_is_gso(skb))
> > > > > > + return -EINVAL;
> > > >
> > > > This is a risk the commit message will have to call out.
> > >
> > > I tried with this:
> > >
> > > > > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > > > > assumes knowledge of the protocol. We could segment ignoring the
> > > > > > checksum then do whatever csum_start and csum_offset say, but that
> > > > > > seems like an overkill for something that should happen.
> > >
> > > I guess I can add a more detailed explanation about what the risk is?
> >
> > Let's just explicitly mention the consequence: non-standard GSO
> > packets that previous would be sent may now fail.
> >
> > >
> > > > I don't see a simple way around it. We discussed segmenting here too,
> > > > but that requires intrusive changes to all callers too.
> > > > Alternatively,
> > > >
> > > > 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> > > > thus verify that the TSO/USO packet is valid. Still drop otherwise.
> > > > This also does not add overhead, because it would be called later
> > > > anyway.
> > > >
> > > > 2. a failsafe off-by-default sysctl + static_branch that skips the
> > > > drop when set. Messy, but so is the situation we're dealing with.
> > >
> > > Tack this on top or do you feel it's needed straight away.
> >
> > Not urgent. Can even wait until a report appears. But worth taking a
> > look sooner if either of us gets around to it.
>
> Maybe if people intend all this for stable, I'll feel safer with this in
> there.
>
> Just to make sure, the branch would just short-circuit all checks, yes?
Ah option 2, the failsafe? At least any new failures (GSO) that could
cause regression reports. Operating entirely as before this series
sounds fine too.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 20:45 ` Willem de Bruijn
@ 2026-10-13 20:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 20:51 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Rusty Russell
On Tue, Oct 13, 2026 at 02:45:07PM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 2:38 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 02:32:26PM -0600, Willem de Bruijn wrote:
> > > > > > > +
> > > > > > > + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> > > > > > > + inner_proto))
> > > > > > > + return 0;
> > > > > > > +
> > > > > > > + if (skb_is_gso(skb))
> > > > > > > + return -EINVAL;
> > > > >
> > > > > This is a risk the commit message will have to call out.
> > > >
> > > > I tried with this:
> > > >
> > > > > > > We can't easily "complete the checksum" for GSO we don't recognize - GSO
> > > > > > > assumes knowledge of the protocol. We could segment ignoring the
> > > > > > > checksum then do whatever csum_start and csum_offset say, but that
> > > > > > > seems like an overkill for something that should happen.
> > > >
> > > > I guess I can add a more detailed explanation about what the risk is?
> > >
> > > Let's just explicitly mention the consequence: non-standard GSO
> > > packets that previous would be sent may now fail.
> > >
> > > >
> > > > > I don't see a simple way around it. We discussed segmenting here too,
> > > > > but that requires intrusive changes to all callers too.
> > > > > Alternatively,
> > > > >
> > > > > 1. call segmentation, but only to see it remove the SKB_GSO_DODGY and
> > > > > thus verify that the TSO/USO packet is valid. Still drop otherwise.
> > > > > This also does not add overhead, because it would be called later
> > > > > anyway.
> > > > >
> > > > > 2. a failsafe off-by-default sysctl + static_branch that skips the
> > > > > drop when set. Messy, but so is the situation we're dealing with.
> > > >
> > > > Tack this on top or do you feel it's needed straight away.
> > >
> > > Not urgent. Can even wait until a report appears. But worth taking a
> > > look sooner if either of us gets around to it.
> >
> > Maybe if people intend all this for stable, I'll feel safer with this in
> > there.
> >
> > Just to make sure, the branch would just short-circuit all checks, yes?
>
> Ah option 2, the failsafe? At least any new failures (GSO) that could
> cause regression reports. Operating entirely as before this series
> sounds fine too.
Right. I feel 1 is just as likely to cause regressions. At least 2 gives
us exactly legacy behaviour back.
--
MST
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 20:36 ` Michael S. Tsirkin
@ 2026-10-13 20:54 ` Willem de Bruijn
2026-10-13 20:56 ` Michael S. Tsirkin
0 siblings, 1 reply; 27+ messages in thread
From: Willem de Bruijn @ 2026-10-13 20:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 2:36 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 13, 2026 at 02:29:53PM -0600, Willem de Bruijn wrote:
> > On Tue, Oct 13, 2026 at 12:41 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Oct 13, 2026 at 08:41:19AM -0600, Willem de Bruijn wrote:
> > > > On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > Allowing untrusted hardware to specify csum start/offset for
> > > > > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > > > > outside the actual checksum, and depending on the setup, the networking
> > > > > stack could be looking at the packet both before and after the edit, and
> > > > > seeing different results.
> > > > >
> > > > > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > > > > completing checksum") validated all known instances where this
> > > > > causes the most grief, but the real fix is to make sure
> > > > > that either only actual checksum fields are ever edited,
> > > > > or the checksum is completed before the net stack looks at it,
> > > > > so it is always consistent.
> > > >
> > > > +1 on the general approach of performing the checksum offload at
> > > > input, unless the input can easily be validated to be one of a few
> > > > common protocols. That greatly reduces the risk vs my previous attempt
> > > > at trying to drop.
> > > >
> > > > Since we do not know the full extent of grief, I think this too should
> > > > go to net and stable.
> > >
> > > I think it needs a bit more testing to be frank, I do not see it
> > > wait months out of the tree. So if this lands in next before the
> > > merge window, we are good I think. If we miss that I will target net.
> > >
> > >
> > > > That would mean initially a single patch that
> > > > touches as few files as possible. The helper function in
> > > > include/linux/ip.h and conversion of existing open coded parsers to
> > > > use that can follow up later in net-next.
> > >
> > > I don't see the value to be frank.
> > > What moving code between files accomplishes, I donnu.
> > > Squashing things surely makes review harder?
> > > Does your backporting system really struggle with trivial
> > > dependencies like this?
> > >
> > > I can drop patch 2 and will if I am targeting net.
> >
> > Good. This is not about "my" backporting, but upstream stable.
> > Refactor patches do not belong there.
> >
> > Since patch 3 might have to be backported in the future if a serious
> > abuse surfaces, even if you now land it in net-next, it is better to
> > not depend on other patches.
>
> Right. While this is targeting next, I will reorder it last in series so
> it's obvious for people it can be dropped.
>
> Thanks a lot for the review!
Of course. Thanks for the fixes!
I would open-code ip4_parse directly in "net: resolve uncommon partial
checksums at ingress". It's not a lot of code. Then that patch stands
on its own.
And then optionally squash patches 1 and 2 and place them at the end
of this series as one dedup patch, or send that separately later.
Personally I would drop that entirely and leave the existing code as
is. But either way.
>
> > > >
> > > > > This basically does this: checksum at ingress, except in a narrow list
> > > > > of specified known good cases.
> > > > >
> > > > > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> > > > >
> > > > > Michael S. Tsirkin (4):
> > > > > net: add ip4_parse()
> > > > > net: use ip4_parse() in receive validation
> > > > > net: resolve uncommon partial checksums at ingress
> > > > > net: account for ingress-resolved virtio net hdr state
> > >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 0/4] more virtio-net csum hardening
2026-10-13 20:54 ` Willem de Bruijn
@ 2026-10-13 20:56 ` Michael S. Tsirkin
0 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2026-10-13 20:56 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-kernel, Jason Wang, Eugenio Pérez, Xuan Zhuo,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, virtualization, Max Krasnyansky,
Paulos Yibelo, Chris Arges
On Tue, Oct 13, 2026 at 02:54:24PM -0600, Willem de Bruijn wrote:
> On Tue, Oct 13, 2026 at 2:36 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 02:29:53PM -0600, Willem de Bruijn wrote:
> > > On Tue, Oct 13, 2026 at 12:41 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Oct 13, 2026 at 08:41:19AM -0600, Willem de Bruijn wrote:
> > > > > On Tue, Oct 13, 2026 at 4:58 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > >
> > > > > > Allowing untrusted hardware to specify csum start/offset for
> > > > > > CHECKSUM_PARTIAL was not a good idea, because, if it edits the packet
> > > > > > outside the actual checksum, and depending on the setup, the networking
> > > > > > stack could be looking at the packet both before and after the edit, and
> > > > > > seeing different results.
> > > > > >
> > > > > > Previously, commit 6785011f8b16 ("ipv4: validate checksum_start before
> > > > > > completing checksum") validated all known instances where this
> > > > > > causes the most grief, but the real fix is to make sure
> > > > > > that either only actual checksum fields are ever edited,
> > > > > > or the checksum is completed before the net stack looks at it,
> > > > > > so it is always consistent.
> > > > >
> > > > > +1 on the general approach of performing the checksum offload at
> > > > > input, unless the input can easily be validated to be one of a few
> > > > > common protocols. That greatly reduces the risk vs my previous attempt
> > > > > at trying to drop.
> > > > >
> > > > > Since we do not know the full extent of grief, I think this too should
> > > > > go to net and stable.
> > > >
> > > > I think it needs a bit more testing to be frank, I do not see it
> > > > wait months out of the tree. So if this lands in next before the
> > > > merge window, we are good I think. If we miss that I will target net.
> > > >
> > > >
> > > > > That would mean initially a single patch that
> > > > > touches as few files as possible. The helper function in
> > > > > include/linux/ip.h and conversion of existing open coded parsers to
> > > > > use that can follow up later in net-next.
> > > >
> > > > I don't see the value to be frank.
> > > > What moving code between files accomplishes, I donnu.
> > > > Squashing things surely makes review harder?
> > > > Does your backporting system really struggle with trivial
> > > > dependencies like this?
> > > >
> > > > I can drop patch 2 and will if I am targeting net.
> > >
> > > Good. This is not about "my" backporting, but upstream stable.
> > > Refactor patches do not belong there.
> > >
> > > Since patch 3 might have to be backported in the future if a serious
> > > abuse surfaces, even if you now land it in net-next, it is better to
> > > not depend on other patches.
> >
> > Right. While this is targeting next, I will reorder it last in series so
> > it's obvious for people it can be dropped.
> >
> > Thanks a lot for the review!
>
> Of course. Thanks for the fixes!
>
> I would open-code ip4_parse directly in "net: resolve uncommon partial
> checksums at ingress". It's not a lot of code. Then that patch stands
> on its own.
Ok if you prefer, except I'll make it a small inline in the same file.
Easier to then dedup at the end.
> And then optionally squash patches 1 and 2 and place them at the end
> of this series as one dedup patch, or send that separately later.
> Personally I would drop that entirely and leave the existing code as
> is. But either way.
>
> >
> > > > >
> > > > > > This basically does this: checksum at ingress, except in a narrow list
> > > > > > of specified known good cases.
> > > > > >
> > > > > > Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> > > > > >
> > > > > > Michael S. Tsirkin (4):
> > > > > > net: add ip4_parse()
> > > > > > net: use ip4_parse() in receive validation
> > > > > > net: resolve uncommon partial checksums at ingress
> > > > > > net: account for ingress-resolved virtio net hdr state
> > > >
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 1/4] net: add ip4_parse()
2026-10-13 10:58 ` [PATCH net-next 1/4] net: add ip4_parse() Michael S. Tsirkin
@ 2026-10-14 10:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-10-14 10:59 UTC (permalink / raw)
To: mst
Cc: linux-kernel, willemdebruijn.kernel, jasowangio, eperezma,
xuanzhuo, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, maxk, rusty, horms
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The doc comment on ip4_parse() (include/linux/ip.h) promises more
than the code checks.
- [Low] API hazard: nhoff is unsigned int and `nhoff + sizeof(struct
iphdr)` is not checked for overflow.
--
net: add ip4_parse()
This commit adds an inline helper, ip4_parse(), to include/linux/ip.h.
It pulls the fixed IPv4 header at a given offset, checks for version 4
and IHL >= 5, and returns the header together with the transport header
offset. It is meant to replace several open-coded copies of the same
minimal validation in the net stack.
> diff --git a/include/linux/ip.h b/include/linux/ip.h
> index d11c25f5030a0..98012f6d0efb5 100644
> --- a/include/linux/ip.h
> +++ b/include/linux/ip.h
> @@ -56,4 +56,28 @@ static inline void iph_set_totlen(struct iphdr *iph, unsigned int len)
> {
> iph->tot_len = len <= IP_MAX_MTU ? htons(len) : 0;
> }
> +
> +/*
> + * Pull and minimally validate the IPv4 header at @nhoff.
> + * Returns the header and stores the transport header offset in
> + * @thoff, or NULL if truncated or malformed. Only the fixed header
> + * is guaranteed linear; callers that need the options or the
> + * transport header must pull further.
> + */
[Severity: Low]
Is "NULL if truncated or malformed" accurate here? ip4_parse() only does
three checks: a pskb_may_pull() of the 20-byte fixed header, version == 4,
and ihl >= 5.
Two kinds of bad header still get a non-NULL iph back:
- A header cut off inside its options, for example ihl = 15 with only
20 bytes in the skb.
- A header whose tot_len is smaller than ihl * 4.
In both cases thoff is stored without being checked against skb->len or
ntohs(iph->tot_len):
*thoff = nhoff + iph->ihl * sizeof(u32);
The next sentence of the comment says the options may not be linear. It
does not say that thoff can point past the end of the packet or the
datagram.
The callers added later in this series do their own checks on thoff, so
none of them overruns today. Those callers are ip_rcv_core(),
br_validate_ipv4(), nf_reject_iphdr_validate() and
virtio_net_csum_partial_ok_at_offset().
Could the comment say that only the fixed header is validated, and that
callers must check thoff against what they can pull and against tot_len?
> +static inline struct iphdr *ip4_parse(struct sk_buff *skb,
> + unsigned int nhoff,
> + unsigned int *thoff)
> +{
> + struct iphdr *iph;
> +
> + if (!pskb_may_pull(skb, nhoff + sizeof(struct iphdr)))
> + return NULL;
> +
> + iph = (struct iphdr *)(skb->data + nhoff);
[Severity: Low]
This isn't a bug in any current caller, but nhoff is an unsigned int and
the sum nhoff + sizeof(struct iphdr) is not checked for overflow.
Suppose a caller passed a negative offset, such as a negative
skb_network_offset(), or a very large one. Could the length passed to
pskb_may_pull() then wrap to a small value, while skb->data + nhoff still
uses the original offset?
No caller in the series passes such an offset:
- ip_rcv_core(), br_validate_ipv4() and nf_reject_iphdr_validate()
pass 0.
- virtio_net_csum_partial_ok() passes ETH_HLEN, a VLAN depth, or a
non-negative skb_network_offset().
The IPv6 branch next to the ip4_parse() call open-codes the same
pskb_may_pull(skb, nhoff + sizeof(...)) pattern.
Would it be worth documenting that nhoff must be in range, or rejecting
an nhoff that wraps?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791887415.git.mst%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 2/4] net: use ip4_parse() in receive validation
2026-10-13 10:58 ` [PATCH net-next 2/4] net: use ip4_parse() in receive validation Michael S. Tsirkin
@ 2026-10-14 10:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-10-14 10:59 UTC (permalink / raw)
To: mst
Cc: linux-kernel, willemdebruijn.kernel, jasowangio, eperezma,
xuanzhuo, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, maxk, rusty, horms
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message overstates how many sites were converted.
--
net: use ip4_parse() in receive validation
This patch converts br_validate_ipv4(), ip_rcv_core() and
nf_reject_iphdr_validate() to use ip4_parse(). They previously did an
open-coded pskb_may_pull() followed by version and ihl checks. The
returned thoff is then used in the later length checks.
> Convert the sites that pull and do version and ihl checks on the ipv4
> header and validate the packet transport offset to use ip4_parse.
>
> Sites that validate a read-only copy instead of pulling - the nft
> and nft_inner header_pointer validators, ip_check_defrag()'s
> skb_copy_bits, the flow dissector - stay open-coded for now.
[Severity: Low]
This isn't a bug, but is this description complete? A few sites still
call pskb_may_pull() and then open-code the version and ihl checks. The
read-only copy exclusion list above does not cover them.
nft_set_pktinfo_ipv4_ingress() in include/net/netfilter/nf_tables_ipv4.h:
if (!pskb_may_pull(pkt->skb, sizeof(*iph)))
return -1;
iph = ip_hdr(pkt->skb);
if (iph->ihl < 5 || iph->version != 4)
goto inhdr_error;
This has the same pattern as br_validate_ipv4(). The header_pointer
validator in that file is __nft_set_pktinfo_ipv4_validate(), not this
function.
ip_mc_check_iphdr() in net/ipv4/igmp.c:
if (!pskb_may_pull(skb, offset))
return -EINVAL;
iph = ip_hdr(skb);
if (iph->version != 4 || ip_hdrlen(skb) < sizeof(*iph))
return -EINVAL;
offset += ip_hdrlen(skb) - sizeof(*iph);
if (!pskb_may_pull(skb, offset))
return -EINVAL;
This looks like it maps directly onto ip4_parse(skb,
skb_network_offset(skb), &thoff).
nf_ct_bridge_pre() in net/bridge/netfilter/nf_conntrack_bridge.c calls
pskb_may_pull(skb, sizeof(struct iphdr)). nf_ct_br_ip_check() then
checks:
if (iph->ihl < 5 ||
iph->version != 4)
return -1;
All three sites are still open-coded at the end of the series, after
"net: account for ingress-resolved virtio net hdr state". At that point
the only ip4_parse() callers are br_validate_ipv4(), ip_rcv_core(),
nf_reject_iphdr_validate() and include/linux/virtio_net.h.
A smaller point: ip_rcv_core() still sets the transport header from ihl,
even though thoff is now in scope:
skb->transport_header = skb->network_header + iph->ihl*4;
This has no functional effect.
Should these sites be converted, or should the commit message list them
as staying open-coded?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791887415.git.mst%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress
2026-10-13 10:58 ` [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress Michael S. Tsirkin
2026-10-13 11:06 ` Michael S. Tsirkin
@ 2026-10-14 10:59 ` netdev-bot+sashiko
1 sibling, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-10-14 10:59 UTC (permalink / raw)
To: mst
Cc: linux-kernel, willemdebruijn.kernel, jasowangio, eperezma,
xuanzhuo, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, maxk, rusty, horms
Thank you for your contribution! Sashiko AI review found 8 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 5 · Low: 2
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] On the UDP-tunnel path, virtio_net_hdr_tnl_to_skb() checks
CHECKSUM_PARTIAL only against the 'inner' IP/TCP/UDP header at…
- [Medium] With this patch applied, virtio_net_hdr_tnl_to_skb() in
include/linux/virtio_net.h ends with `ret = skb_checksum_help(skb);…
- [Medium] On UML, the new skb_checksum_help() fallback in
__virtio_net_hdr_to_skb() checksums the whole preallocated receive…
- [Medium] On UML, the patch's new rejection of GSO packets with
unrecognised checksum metadata never takes effect.
- [Medium] On any device whose type is not ARPHRD_ETHER,
virtio_net_csum_partial_ok() takes the L3 protocol only from…
- [Medium] For non-ARPHRD_ETHER devices that still carry Ethernet
framing, chiefly loopback (ARPHRD_LOOPBACK, hard_header_len =…
- [Low] The commit "net: resolve uncommon partial checksums at ingress"
is the actual fix for user-reachable checksum-metadata abuse, but it…
- [Low] The commit message does not state the user-visible accept/reject
changes precisely, and it contains a typo that inverts its meaning.
--
net: resolve uncommon partial checksums at ingress
This commit validates the virtio net header csum_start and csum_offset at
ingress. CHECKSUM_PARTIAL is kept only when they match a parsed TCP or UDP
header behind Ethernet/VLAN and IPv4/IPv6. Other non-GSO partial checksums
are completed at once with skb_checksum_help(), unrecognized GSO is
rejected, and the UDP tunnel path checks the inner header the same way.
> We can't easily "complete the checksum" for GSO we don't recognize - GSO
> assumes knowledge of the protocol. We could segment ignoring the
> checksum then do whatever csum_start and csum_offset say, but that
> seems like an overkill for something that should happen.
[Severity: Low]
This isn't a bug, but should "something that should happen" read
"something that shouldn't happen"?
Could the commit message also spell out the accept/reject changes?
This patch removes two checks from __virtio_net_hdr_to_skb(): the
skb_transport_offset(skb) < nh_min_len check and the per-GSO-type
csum_offset checks. Non-GSO packets that used to fail with -EINVAL (and
were counted in tun's rx_frame_errors) are now accepted after
skb_checksum_help().
In the other direction, GSO packets are now rejected unless csum_start
matches a transport offset the kernel can parse. On non-ARPHRD_ETHER
devices that parse depends on skb->protocol and skb_network_offset().
The old code never used either of them.
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd00..b181183118b3a 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
[ ... ]
> @@ -48,15 +52,111 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
[ ... ]
> +static inline bool
> +virtio_net_csum_partial_ok(struct sk_buff *skb, u8 hdr_gso_type)
> +{
> + unsigned int nhoff;
> + __be16 proto;
> +
> + if (skb->dev->type == ARPHRD_ETHER) {
> + if (!pskb_may_pull(skb, ETH_HLEN))
> + return false;
> + nhoff = ETH_HLEN;
> + proto = eth_hdr(skb)->h_proto;
> + if (eth_type_vlan(proto)) {
> + proto = __vlan_get_protocol(skb, proto, &nhoff);
> + if (!proto)
> + return false;
> + }
> + } else {
> + nhoff = skb_network_offset(skb);
> + proto = skb->protocol;
> + }
[Severity: Medium]
Can this reject valid NEEDS_CSUM GSO packets sent through AF_PACKET on
devices that aren't ARPHRD_ETHER? Examples are loopback, tun in IFF_TUN
mode, ipip/gre/sit and wireguard.
In packet_snd(), skb->protocol is only set when the socket protocol isn't
ETH_P_ALL, so it stays 0 for protocol-0 sockets. The protocol is only
derived from the headers after the vnet header has been converted:
net/packet/af_packet.c:packet_snd() {
...
if (proto != htons(ETH_P_ALL))
skb->protocol = proto;
...
if (vnet_hdr_sz) {
err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
if (err)
goto out_free;
len += vnet_hdr_sz;
}
packet_parse_headers(skb, sock);
if (vnet_hdr_sz)
virtio_net_hdr_set_proto(skb, &vnet_hdr);
...
}
tpacket_snd() uses the same order.
When skb->protocol is 0, virtio_net_csum_partial_ok_at_offset() falls
into its final "else return false", even for a well-formed IPv4/TCP
packet. __virtio_net_hdr_to_skb() then returns -EINVAL for any GSO
packet, so sendmsg() rejects TSO packets that it used to accept.
Non-GSO packets always get a software checksum.
The non-NEEDS_CSUM GSO branch of __virtio_net_hdr_to_skb() already
handles a zero protocol:
if (!skb->protocol) {
skb->protocol = dev_parse_header_protocol(skb);
if (!skb->protocol)
virtio_net_hdr_set_proto(skb, hdr);
}
Should the new check have a similar fallback?
[Severity: Medium]
A related question: is skb->protocol a reliable source for devices that
use Ethernet framing but aren't ARPHRD_ETHER? The main case is loopback,
which is ARPHRD_LOOPBACK with hard_header_len == ETH_HLEN.
In packet_snd(), skb->protocol comes from the user's sll_protocol.
loopback_xmit() then recomputes it from the frame bytes:
drivers/net/loopback.c:loopback_xmit() {
...
skb->protocol = eth_type_trans(skb, dev);
...
}
A CAP_NET_RAW sender, possibly inside a user namespace, could set
sll_protocol to ETH_P_IP while h_proto names a different encapsulation
such as MPLS. This check would then accept CHECKSUM_PARTIAL against a
fake IPv4/TCP header at offset 14. The receive path would parse a
different layout, so csum_start could end up inside the real transport
header.
The ARPHRD_ETHER branch avoids this by parsing h_proto itself. Nobody has
shown this can be exploited, so it is speculative. Would parsing the
frame be safer for Ethernet-framed devices too?
[ ... ]
> @@ -104,11 +203,11 @@ 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)
> - return -EINVAL;
> + if (need_csum_check)
> + csum_partial_ok =
> + virtio_net_csum_partial_ok(skb, hdr_gso_type);
[Severity: Low]
This replaces the weak skb_transport_offset(skb) < nh_min_len check.
Should the commit carry a Fixes: tag and Cc: stable?
Before this patch, csum_offset was only checked inside the GSO switch.
A non-GSO NEEDS_CSUM packet from a tun/tap user, a guest or an AF_PACKET
sender could therefore put the deferred checksum write almost anywhere
past byte 20, including in IP header fields.
The only Fixes: tag in the series is on the later commit "net: account
for ingress-resolved virtio net hdr state". That commit adds a counter
and does not contain this fix.
[ ... ]
> @@ -191,6 +284,17 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
> }
> }
>
> + if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && !csum_partial_ok) {
> + int err;
> +
> + if (skb_is_gso(skb))
> + return -EINVAL;
[Severity: Medium]
Does this rejection take effect for the UML vector driver?
raw_verify_header() ignores the return value:
arch/um/drivers/vector_transports.c:raw_verify_header() {
...
skb_set_network_header(skb, ETH_HLEN);
virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
return 0;
}
By the time -EINVAL is returned here, skb_partial_csum_set() has already
set CHECKSUM_PARTIAL at the offset from the header. The GSO block has
also set gso_size and gso_type. This happens more often on UML because
skb->len is still the untrimmed buffer length.
vector_legacy_rx() and vector_mmsg_rx() only drop the packet when
verify_header() returns a negative value:
arch/um/drivers/vector_kern.c:vector_legacy_rx() {
...
if (header_check < 0) {
dev_kfree_skb_irq(skb);
...
}
So the skb still reaches eth_type_trans() and napi_gro_receive() with
CHECKSUM_PARTIAL at an arbitrary offset. The later patch in the series
doesn't change this caller.
Should raw_verify_header() propagate the error?
> +
> + err = skb_checksum_help(skb);
> + skb->transport_header = old_transport;
> + return err;
> + }
[Severity: Medium]
Is skb->len the real packet length here for every caller? On UML,
prep_skb() sizes the skb to the whole receive buffer:
arch/um/drivers/vector_kern.c:prep_skb() {
...
skb_put(result, vp->max_packet);
result->data_len = len - vp->max_packet;
result->len += len - vp->max_packet;
...
}
and vector_legacy_rx() only trims the skb after verify_header():
arch/um/drivers/vector_kern.c:vector_legacy_rx() {
...
header_check = vp->verify_header(
vp->header_rxbuffer, skb, vp);
...
pskb_trim(skb, pkt_len - vp->rx_header_size);
...
}
vector_mmsg_rx() has the same order.
Take a non-GSO NEEDS_CSUM frame that virtio_net_csum_partial_ok() doesn't
recognize, for example tunneled traffic with an inner partial checksum.
skb_checksum_help() sums skb->len - offset bytes, which includes stale
bytes at the end of the buffer. It writes the folded result into the
packet and sets CHECKSUM_NONE. The later pskb_trim() doesn't correct the
checksum.
Before this patch these packets kept CHECKSUM_PARTIAL and were trimmed
correctly. Could they now be dropped with a bad L4 checksum, or forwarded
with a value computed from stale memory? The later patch in the series
doesn't change arch/um.
[ ... ]
> @@ -334,7 +440,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>
> /* 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, false);
> if (ret)
> return ret;
>
[ ... ]
> @@ -368,7 +474,16 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> skb->inner_mac_header = inner_nh + skb_headroom(skb);
> skb->transport_header = outer_th + skb_headroom(skb);
> skb->encapsulation = 1;
> - return 0;
> +
> + if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh,
> + inner_proto))
> + return 0;
[Severity: High]
Is it enough to anchor the check at inner_nh? inner_nh is taken straight
from vhdr->inner_nh_offset. Because need_csum_check is false in the call
above, the Ethernet/VLAN/IP parse from the start of the frame is skipped.
The outer offsets are only checked against each other:
include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() {
...
if (outer_th < outer_l3min ||
inner_nh < outer_th + sizeof(struct udphdr) ||
inner_th < inner_nh + inner_l3min)
return -EINVAL;
...
}
Nothing checks that the real outer IP header is followed by UDP at
outer_th. Nothing checks that VLAN tags, IPv4 options or IPv6 extension
headers don't move the real transport header either.
Two senders can control these offsets:
- a tun user with the tunnel vnet header, through tun_get_user() or
tun_xdp_one() via tun_vnet_hdr_tnl_to_skb()
- a host feeding a virtio_net guest with rx_tnl negotiated
Either one could put a fake IPv4/IPv6 plus TCP/UDP header at any
inner_nh and keep CHECKSUM_PARTIAL there. That includes inside IPv4
options, inside IPv6 extension headers, or in the outer L4 payload.
For small packets gso_size is never set, so a non-GSO skb with
CHECKSUM_PARTIAL and encapsulation set is accepted. A later
skb_checksum_help() in validate_xmit_skb(), or NIC offload after
forwarding, would then write 2 bytes at a place that need not match the
headers the stack parsed. That could be the real outer L4 port.
Doesn't this leave open the case the commit message describes ("later
checksum completion can modify the packet after protocol processing has
made decisions from it")? It also seems to fall short of "UDP tunnel
headers get the same handling on the inner header".
Would the outer headers need to be parsed from the start of the frame as
well?
> +
> + if (skb_is_gso(skb))
> + return -EINVAL;
> +
> + ret = skb_checksum_help(skb);
> + return ret ?: 1;
[Severity: Medium]
What do callers do with the 1 returned here after skb_checksum_help()
succeeds? At this revision every caller treats any non-zero value as an
error. For example, in virtnet_receive_done():
drivers/net/virtio_net.c:virtnet_receive_done() {
...
if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
vi->rx_tnl_csum,
virtio_is_little_endian(vi->vdev))) {
...
goto frame_err;
}
...
}
tun_get_user() and tun_xdp_one() do the same through
tun_vnet_hdr_tnl_to_skb():
drivers/net/tun.c:tun_get_user() {
...
if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
atomic_long_inc(&tun->rx_frame_errors);
err = -EINVAL;
goto free_skb;
}
...
}
As a result, non-GSO tunnel packets whose inner checksum metadata isn't
recognized are dropped and counted as frame errors instead of being
accepted.
The later commit "net: account for ingress-resolved virtio net hdr state"
in this series changes these callers to test for < 0 and documents the
return value of 1. Could that change be folded into this patch so the
series stays bisectable?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791887415.git.mst%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state
2026-10-13 10:58 ` [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
2026-10-13 17:45 ` Björn Töpel
@ 2026-10-14 10:59 ` netdev-bot+sashiko
1 sibling, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-10-14 10:59 UTC (permalink / raw)
To: mst
Cc: linux-kernel, willemdebruijn.kernel, jasowangio, eperezma,
xuanzhuo, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, maxk, rusty, horms
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 1
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).
- [Medium] This patch silently fixes a packet-drop regression introduced
by the previous commit in the series, "net: resolve uncommon partial…
- [Medium] tun_get_ethtool_stats() assigns `data[0] =
atomic_long_read(&tun->rx_csum_sw);` straight into the u64 ethtool…
- [Low] The trailer `Fixes: f43798c27684 ("tun: Allow GSO using
virtio_net_hdr")` names a July 2008 commit by Rusty Russell (verified…
--
net: account for ingress-resolved virtio net hdr state
After this commit, virtio_net_hdr_to_skb() returns 1 when it completes an
uncommon checksum in software at ingress. The converter call sites in tap,
tun, virtio_net and af_packet now test for a negative errno. tun counts
these events per device as rx_csum_sw and exposes the counter through
ethtool -S.
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
[Severity: Low]
Is this the right Fixes: target? f43798c27684 is the 2008 commit that
added GSO via virtio_net_hdr to tun.
Software checksum completion in the converter first appears in the
previous patch in this series, "net: resolve uncommon partial checksums
at ingress". So does the positive return value this patch accounts for.
At the baseline there is no skb_checksum_help() call in
include/linux/virtio_net.h.
Stable tooling that follows this tag could offer the patch to every
stable branch since 2.6.27. On those branches the "return err ?: 1"
hunk has nothing to apply to, and rx_csum_sw would always read 0.
Should the tag be dropped? If it is meant to cover the call site fix
below, should it point at the previous commit in this series instead?
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 8b82a7fe6ec7f..b8961229737fd 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
[ ... ]
> @@ -1935,11 +1936,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> break;
> }
>
> - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
> + err = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr);
> + if (err < 0) {
> atomic_long_inc(&tun->rx_frame_errors);
> err = -EINVAL;
> goto free_skb;
> }
[Severity: Medium]
Does this change also fix a regression from the previous patch in the
series, "net: resolve uncommon partial checksums at ingress"?
That commit already made virtio_net_hdr_tnl_to_skb() return 1 on the
UDP tunnel path after a successful software checksum:
include/linux/virtio_net.h:virtio_net_hdr_tnl_to_skb() {
...
ret = skb_checksum_help(skb);
return ret ?: 1;
}
At that commit, the tunnel converter callers still treated any nonzero
return as a failure:
tun_get_user():
if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
atomic_long_inc(&tun->rx_frame_errors);
err = -EINVAL;
goto free_skb;
}
tun_xdp_one() had the same pattern. virtnet_receive_done() went to
frame_err and printed a "bad gso" warning.
Take a UDP tunnel header with these properties:
- NEEDS_CSUM is set.
- The payload is no larger than gso_size, so skb_is_gso() is false.
- The inner csum offsets fail virtio_net_csum_partial_ok_at_offset().
With only the previous commit applied, such a packet would have its
checksum completed and then be dropped as a frame error. tun write()
would return -EINVAL for it.
This commit message describes the "< 0" changes only as accounting
plumbing. To keep the series bisectable, could the call site changes be
folded into "net: resolve uncommon partial checksums at ingress"?
Alternatively, could this commit message say that it fixes that commit?
> + if (err > 0)
> + atomic_long_inc(&tun->rx_csum_sw);
[ ... ]
> @@ -3739,7 +3751,35 @@ static void tun_get_channels(struct net_device *dev,
[ ... ]
> +static void tun_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> +
> + data[0] = atomic_long_read(&tun->rx_csum_sw);
[Severity: Medium]
On 32-bit builds, atomic_long_t is an atomic_t and atomic_long_read()
returns a signed long. Once rx_csum_sw passes 2^31, can the implicit
conversion to u64 sign-extend the value? ethtool -S would then report
something like 0xffffffff80000000.
tun_net_get_stats64() in the same file avoids this for the sibling
counter:
stats->rx_frame_errors +=
(unsigned long)atomic_long_read(&tun->rx_frame_errors);
Should this read use the same (unsigned long) cast?
> +}
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791887415.git.mst%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2026-10-14 10:59 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-13 10:58 [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
2026-10-13 10:58 ` [PATCH net-next 1/4] net: add ip4_parse() Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 2/4] net: use ip4_parse() in receive validation Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 3/4] net: resolve uncommon partial checksums at ingress Michael S. Tsirkin
2026-10-13 11:06 ` Michael S. Tsirkin
2026-10-13 15:03 ` Willem de Bruijn
2026-10-13 18:35 ` Michael S. Tsirkin
2026-10-13 20:32 ` Willem de Bruijn
2026-10-13 20:38 ` Michael S. Tsirkin
2026-10-13 20:45 ` Willem de Bruijn
2026-10-13 20:51 ` Michael S. Tsirkin
2026-10-13 20:15 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 10:58 ` [PATCH net-next 4/4] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
2026-10-13 17:45 ` Björn Töpel
2026-10-13 18:12 ` Michael S. Tsirkin
2026-10-14 10:59 ` netdev-bot+sashiko
2026-10-13 11:05 ` [PATCH net-next 0/4] more virtio-net csum hardening Michael S. Tsirkin
2026-10-13 18:51 ` Chris Arges
2026-10-13 14:41 ` Willem de Bruijn
2026-10-13 18:41 ` Michael S. Tsirkin
2026-10-13 20:29 ` Willem de Bruijn
2026-10-13 20:36 ` Michael S. Tsirkin
2026-10-13 20:54 ` Willem de Bruijn
2026-10-13 20:56 ` Michael S. Tsirkin
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®