Re: [PATCH net v4 1/2] net: validate virtio checksum start after network header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: virtio_net_hdr_to_skb security check bypassed for TUN devices
--

commit 1c7e0adac42db0550277cf456f6845d512993ef1
Author: Paulos Yibelo <habte.yibelo@gmail.com>

net: validate virtio checksum start after network header

__virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than an
estimated minimum network-header length. The comparison currently uses the
offset from skb->data rather than the offset from skb_network_header().

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index c381b916c1b54..a95ad465b9887 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
[ ... ]
> @@ -104,7 +104,8 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  
>  		if (!skb_partial_csum_set(skb, start, off))
>  			return -EINVAL;
> -		if (skb_transport_offset(skb) < nh_min_len)
> +		if (skb_transport_offset(skb) < nh_min_len ||
> +		    skb_transport_offset(skb) - skb_network_offset(skb) < nh_min_len)

[Severity: High]
Does this check get bypassed for TUN/TAP devices?

Looking at drivers/net/tun.c:tun_get_user(), the virtio_net_hdr parsing is
invoked before the network header is reset:

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;
	}
	...
	skb_reset_network_header(skb);
	...
}

Because skb->network_header is uninitialized when __virtio_net_hdr_to_skb()
is called, it retains its 0 value from allocation. This causes
skb_network_offset(skb) to evaluate to a negative value, such as -64 for
prepad bytes.

The new check then simplifies to start + prepad < nh_min_len. Since prepad
is typically >= 32 and nh_min_len is usually around 20, this comparison is
always false. This completely bypasses the security check and leaves TUN/TAP
devices vulnerable to the memory corruption this patch intends to fix.

While a subsequent patch in the series mitigates the consequence for IPv4 in
ip_do_fragment(), are other paths like IPv6 fragmentation via ip6_fragment()
still fully exposed to this bypass?

>  			return -EINVAL;
>  
>  		nh_min_len = skb_transport_offset(skb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920004733.6473-1-habte.yibelo@gmail.com?part=1