Message-ID: <sashiko-outbox-182806@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH net v5 5/7] net: add a sysctl to opt out of virtio net
 hdr checksum validation
Reply-To: sashiko-reviews@lists.linux.dev
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux.dev, "Eugenio Perez" <eperezma@redhat.com>
In-Reply-To: <2beba4c8b84c811120b6100bcccd8b90e7458787.1792668082.git.mst@redhat.com>
References: <cover.v5.git.mst@redhat.com>
 <2beba4c8b84c811120b6100bcccd8b90e7458787.1792668082.git.mst@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Fri, 23 Oct 2026 12:59:07 +0000
Precedence: list

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [High] net: use-after-free in virtio_net_hdr_validate_disabled() due to m=
issing RCU
--

commit 50d6e871df0af42e23190bbc8209691b9cfd76e4
Author: Michael S. Tsirkin <mst@redhat.com>

net: add a sysctl to opt out of virtio net hdr checksum validation

This commit adds a per-netns sysctl, net.core.virtio_net_hdr_validate,
guarded by a static key to allow network namespaces to opt out of virtio
net header checksum validation on ingress.

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 52852a33a2db7..cea70ccf0ed4b 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -5,6 +5,7 @@
>  #include <linux/if_vlan.h>
>  #include <linux/ip.h>
>  #include <linux/ipv6.h>
> +#include <linux/jump_label.h>
>  #include <linux/netdevice.h>
>  #include <linux/udp.h>
>  #include <net/gso.h>
> @@ -78,6 +79,20 @@ static inline struct iphdr *virtio_net_ip4_parse(struc=
t sk_buff *skb,
>  	*thoff =3D nhoff + iph->ihl * sizeof(u32);
>  	return iph;
>  }
> +/* Defined in net/core/dev.c, enabled while any network namespace
> + * has net.core.virtio_net_hdr_validate set to 0.
> + */
> +DECLARE_STATIC_KEY_FALSE(virtio_net_hdr_validate_disabled_key);
> +
> +/* Failsafe: a namespace can disable the check with
> + * net.core.virtio_net_hdr_validate.
> + * The caller must keep the netns of skb->dev alive.
> + */
> +static inline bool virtio_net_hdr_validate_disabled(struct sk_buff *skb)
> +{
> +	return static_branch_unlikely(&virtio_net_hdr_validate_disabled_key) &&
> +	       !READ_ONCE(dev_net(skb->dev)->core.sysctl_virtio_net_hdr_validat=
e);
> +}

[Severity: High]
Can dereferencing dev_net(skb->dev) here without rcu_read_lock() and
dev_net_rcu() cause a use-after-free on struct net when
virtio_net_hdr_validate_disabled_key is enabled?

Callers such as tun_get_user() in drivers/net/tun.c, as well as
tpacket_snd() and packet_snd() in net/packet/af_packet.c, invoke
virtio_net_hdr_tnl_to_skb() or virtio_net_hdr_to_skb() from preemptible
process context without holding rcu_read_lock() or rtnl_lock():

drivers/net/tun.c:tun_get_user() {
    ...
    err =3D tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr);
    ...
}

net/packet/af_packet.c:tpacket_snd() {
    ...
    if (has_vnet_hdr) {
        if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()) < 0) {
            tp_len =3D -EINVAL;
            goto tpacket_error;
        }
    }
    ...
}

net/packet/af_packet.c:packet_snd() {
    ...
    if (vnet_hdr_sz) {
        err =3D virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
        if (err < 0)
            goto out_free;
        len +=3D vnet_hdr_sz;
    }
    ...
}

In these paths, the caller only holds dev_hold(skb->dev) and a reference
to the socket's netns via sock_net(sk). However, struct net_device does
not hold a reference to its struct net, and skb->dev may reside in a
different network namespace from sock_net(sk) (for instance, when a tun
device or target net_device has been moved to another namespace).

Because __dev_change_net_namespace() in net/core/dev.c switches
dev->nd_net via dev_net_set(dev, net) without waiting for existing
dev_hold(dev) references to drop (synchronizing only via
synchronize_net() and rcu_barrier()), a preempted task in tun_get_user(),
tpacket_snd(), or packet_snd() can load dev_net(skb->dev) just before the
device is moved out and that network namespace is destroyed via
cleanup_net() -> net_complete_free(), and then dereference the freed
struct net upon resuming.

For comparison, __skb_flow_dissect() in net/core/flow_dissector.c (which
is called from __virtio_net_hdr_to_skb() in the same paths) takes
rcu_read_lock() and uses dev_net_rcu(skb->dev) (since commit afec62cd0a41).

Should virtio_net_hdr_validate_disabled() likewise guard the lookup with
rcu_read_lock() / rcu_read_unlock() (or scoped_guard(rcu)) and use
dev_net_rcu(skb->dev)?

[ ... ]

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.v5.git.mst@re=
dhat.com?part=3D5