Re: [PATCH net v3 2/8] net: virtio: do not use dev->type to find the outer L3 header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: virtio: outer_l3min includes ETH_HLEN in
  virtio_net_hdr_tnl_to_skb()
--

commit b246747b62fc4e9502b5e1a6192ecf203f97d1f4
Author: Eric Dumazet <edumazet@kernel.org>

net: virtio: do not use dev->type to find the outer L3 header

This commit adds a has_eth parameter to virtio_net_hdr_tnl_to_skb() and
virtio_net_l3_start() so callers specify whether the frame begins with an
Ethernet header instead of relying on skb->dev->type.

Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/

> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 357116c2a1489..b7b6b542f4763 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h

[ ... ]

> @@ -452,6 +450,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
>  static inline int
>  virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,
> +			  bool has_eth,
>  			  bool tnl_hdr_negotiated,
>  			  bool tnl_csum_negotiated,
>  			  bool little_endian)
> @@ -505,7 +504,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  	if (ret)
>  		return ret;
>  
> -	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
> +	if (!virtio_net_tnl_offsets_ok(skb, has_eth, outer_isv6, outer_th))
>  		return -EINVAL;

[Severity: Medium]
When tun_get_user() processes an IFF_TUN device, it resets the network header
offset to 0 and calls tun_vnet_hdr_tnl_to_skb() -> virtio_net_hdr_tnl_to_skb()
with has_eth set to false.

However, earlier in virtio_net_hdr_tnl_to_skb(), outer_l3min still
unconditionally includes ETH_HLEN:

	outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6);

	inner_th = __virtio16_to_cpu(little_endian, hdr->csum_start);
	inner_nh = le16_to_cpu(vhdr->inner_nh_offset);
	outer_th = le16_to_cpu(vhdr->outer_th_offset);
	if (outer_th < outer_l3min ||
	    inner_nh < outer_th + sizeof(struct udphdr) ||
	    inner_th < inner_nh + inner_l3min)
		return -EINVAL;

For an IFF_TUN packet without outer IPv4 options or IPv6 extension headers,
virtio_net_tnl_offsets_ok() -> virtio_net_l3_start() sets nhoff to 0, and
virtio_net_l4_start() computes thoff as 20 (IPv4) or 40 (IPv6), requiring
outer_th == thoff. Because outer_th < outer_l3min rejects outer_th < 34 for
IPv4 or outer_th < 54 for IPv6, won't valid UDP-tunneled GSO packets on
IFF_TUN devices be rejected with -EINVAL unless outer_l3min only adds
ETH_HLEN when has_eth is true?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105115715.1466924-1-edumazet@kernel.org?part=2