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

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

On Thu, Nov 05, 2026 at 12:57:09PM +0100, Eric Dumazet wrote:
> virtio_net_l3_start() decides from skb->dev->type whether the frame
> starts with an Ethernet header. tun decides it from the IFF_TAP and
> IFF_TUN flags, and TUNSETLINK can set the type of a down tun device
> to any value, without changing the flags.
> 
> On an IFF_TAP device set to ARPHRD_NONE, the outer protocol checks
> look at the IP header at the network header (ETH_HLEN), and never read
> the Ethernet header. eth_type_trans() then sets skb->protocol from an
> unchecked h_proto, for instance ETH_P_NSH, and the packet can reach
> the skb_under_panic() addressed by the previous patch.
> 
> On an IFF_TUN device set to ARPHRD_ETHER, the checks validate an
> Ethernet header that tun then treats as the start of the L3 header.
> 
> Checking in tun that the device type matches the framing is not
> enough: the type is read again later, and a concurrent TUNSETLINK can
> change it in between, since tun_get_user() only drops packets of a
> down device after these checks.
> 
> Add a has_eth argument to virtio_net_hdr_tnl_to_skb(), passed down to
> virtio_net_l3_start(), so the device type is no longer used:
> 
> - virtio_net always receives Ethernet frames.
> - tun_get_user() passes whether the device is IFF_TAP.
> - tun_xdp_one() always calls eth_type_trans() on its frames.
> 
> Fixes: 288f30435132 ("tun: enable gso over UDP tunnel support.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/sashiko-outbox-198573@kernel.org/
> Link: https://lore.kernel.org/netdev/179387332014.488856.9111956413610107951@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/tun.c          |  9 +++++++--
>  drivers/net/tun_vnet.h     |  5 +++--
>  drivers/net/virtio_net.c   |  2 +-
>  include/linux/virtio_net.h | 27 +++++++++++++--------------
>  4 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index dcec7311e601..3920b3dff1e8 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1944,7 +1944,8 @@ 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)) {
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr,
> +				    (tun->flags & TUN_TYPE_MASK) == IFF_TAP)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		err = -EINVAL;
>  		goto free_skb;
> @@ -2621,7 +2622,11 @@ 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)) {
> +	/* Frames always start with an Ethernet header here, as the
> +	 * eth_type_trans() call below expects.
> +	 */
> +	if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr,
> +				    true)) {
>  		atomic_long_inc(&tun->rx_frame_errors);
>  		kfree_skb(skb);
>  		ret = -EINVAL;
> diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
> index f4c652b1fa44..3364e08dd6e0 100644
> --- a/drivers/net/tun_vnet.h
> +++ b/drivers/net/tun_vnet.h
> @@ -197,9 +197,10 @@ static inline netdev_features_t tun_vnet_hdr_guest_features(int vnet_hdr_sz)
>  static inline int
>  tun_vnet_hdr_tnl_to_skb(unsigned int flags, netdev_features_t features,
>  			struct sk_buff *skb,
> -			const struct virtio_net_hdr_v1_hash_tunnel *hdr)
> +			const struct virtio_net_hdr_v1_hash_tunnel *hdr,
> +			bool has_eth)
>  {
> -	return virtio_net_hdr_tnl_to_skb(skb, hdr,
> +	return virtio_net_hdr_tnl_to_skb(skb, hdr, has_eth,
>  				features & NETIF_F_GSO_UDP_TUNNEL,
>  				features & NETIF_F_GSO_UDP_TUNNEL_CSUM,
>  				tun_vnet_is_little_endian(flags));
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index dd2c41ef6fbe..ed20e76fbc4c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2533,7 +2533,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>  
>  	skb->dev = dev;
>  	skb_set_network_header(skb, ETH_HLEN);
> -	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
> +	if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, true, vi->rx_tnl,
>  				      vi->rx_tnl_csum,
>  				      virtio_is_little_endian(vi->vdev))) {
>  		net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n",
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 357116c2a148..b7b6b542f476 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -87,20 +87,18 @@ static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
>  }
>  
>  /* Where does the L3 header start, and what protocol is it?
> - * Handles Ethernet and VLAN headers; other devices use the
> - * network header offset the caller set up.
> + * With @has_eth, the frame starts with an Ethernet header, possibly
> + * followed by VLAN tags. Otherwise the L3 header starts at the network
> + * header offset the caller set up.
> + *
> + * The caller knows how the frame will be parsed later: the device
> + * type does not tell, as tun can change it with TUNSETLINK.
>   */
>  static inline bool
> -virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
> +virtio_net_l3_start(struct sk_buff *skb, bool has_eth, int *nhoff,
> +		    __be16 *proto)
>  {
> -	/* skb->dev should always be set here already,
> -	 * but let's not crash when it is not.
> -	 */
> -	if (WARN_ON_ONCE(!skb->dev))
> -		return false;
> -
> -	if (skb->dev->type == ARPHRD_ETHER ||
> -	    skb->dev->type == ARPHRD_LOOPBACK) {
> +	if (has_eth) {
>  		__be16 h_proto;
>  
>  		if (!pskb_may_pull(skb, ETH_HLEN))
> @@ -431,7 +429,7 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
>   * carries, with UDP following it at @outer_th.
>   */
>  static inline bool
> -virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
> +virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool has_eth, bool outer_isv6,
>  			  unsigned int outer_th)
>  {
>  	int nhoff;
> @@ -439,7 +437,7 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
>  	unsigned int thoff;
>  	int ip_proto;
>  
> -	if (!virtio_net_l3_start(skb, &nhoff, &proto))
> +	if (!virtio_net_l3_start(skb, has_eth, &nhoff, &proto))
>  		return false;
>  	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
>  		return false;
> @@ -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;
>  
>  	/* In case of USO, the inner protocol is still unknown and
> -- 
> 2.53.0