Re: [PATCH net] net: extend virtio_net_hdr csum_start checks to VLAN, IPv6 and IP options

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

On Wed, Oct 07, 2026 at 12:36:59PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> __virtio_net_hdr_to_skb() validates hdr->csum_start against nh_min_len:
> 
>     if (skb_transport_offset(skb) < nh_min_len)
>         return -EINVAL;
> 
> Extend the check to account for the link layer header including VLAN
> tags, IPv4 options, and IPv6 other than VIRTIO_NET_HDR_GSO_TCPV6.
> 
> Payload, gso_type and skb->protocol can come from userspace, so cannot
> be trusted to be consistent, or correct.
> 
> Therefore:
> - For Ethernet packets (ARPHRD_ETHER), parse from ETH_HLEN and
>   eth_hdr(skb)->h_proto, advancing past any VLAN tags with
>   __vlan_get_protocol().
> - For non-Ethernet packets, use skb_network_offset(skb) as nhoff and
>   infer the L3 protocol from iph->version at skb->data + nhoff.
> - If skb->protocol is set and disagrees with the protocol parsed from
>   the packet, enforce the minimum header length of both.
> - For non-IP protocols, require only nhoff + nh_min_len. No in-tree
>   non-IP protocol generates CHECKSUM_PARTIAL itself. They only carry it
>   when encapsulating IP (e.g., MPLS), in which case csum_start lies
>   beyond an inner IP header.
> 
> Reported-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Link: https://lore.kernel.org/netdev/20260922030310.8684-2-habte.yibelo@gmail.com/
> Fixes: 49d14b54a527 ("net: test for not too small csum_start in virtio_net_hdr_to_skb()")
> Co-developed-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Signed-off-by: Paulos Yibelo <habte.yibelo@gmail.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

This doesn't fix all the issues, or does it? If not, I am confused why we
are doing this piecemeal approach. I thought we agreed to

a. validate some basic things about the checksum in the core ip stack

b. in virtio, validate specific checksum values
   and for anything else, fill in the checksum and fragment then
   and there


> ---
>  include/linux/virtio_net.h | 48 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index d6466f96cdd0..6a30f58d9d65 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -48,6 +48,52 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
>  	return 0;
>  }
>  
> +static inline bool virtio_net_hdr_thoff_valid(const struct sk_buff *skb,
> +					      unsigned int nh_min_len)
> +{
> +	int thoff = skb_transport_offset(skb);
> +	const struct iphdr *iph;
> +	__be16 proto = 0;
> +	int nhoff;
> +
> +	DEBUG_NET_WARN_ON_ONCE(skb->mac_len);
> +
> +	if (skb->dev->type == ARPHRD_ETHER) {
> +		if (unlikely(thoff < 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);
> +	}
> +
> +	if (unlikely(thoff < nhoff + nh_min_len))
> +		return false;
> +
> +	iph = (const void *)(skb->data + nhoff);
> +	if (!proto) {
> +		if (iph->version == 4)
> +			proto = htons(ETH_P_IP);
> +		else if (iph->version == 6)
> +			proto = htons(ETH_P_IPV6);
> +	}
> +
> +	if (proto == htons(ETH_P_IP) || skb->protocol == htons(ETH_P_IP)) {
> +		if (unlikely(iph->ihl < 5))
> +			return false;
> +		nh_min_len = max_t(u32, iph->ihl * 4, nh_min_len);
> +	}
> +	if (proto == htons(ETH_P_IPV6) || skb->protocol == htons(ETH_P_IPV6))
> +		nh_min_len = max_t(u32, sizeof(struct ipv6hdr), nh_min_len);
> +
> +	return thoff >= nhoff + nh_min_len;
> +}
> +
>  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)
> @@ -104,7 +150,7 @@ 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 (!virtio_net_hdr_thoff_valid(skb, nh_min_len))
>  			return -EINVAL;
>  
>  		nh_min_len = skb_transport_offset(skb);
> -- 
> 2.56.0.360.g66cac248cb-goog