Re: [PATCH net 1/2] net: packet: fix skb->protocol not updated after VLAN network header adjustment

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Overwriting `skb->protocol` with the inner protocol while leaving the VLAN tag inline in the payload hides the VLAN tag from the network stack and drivers. This breaks packet transmission for VLAN-tagged packets sent via AF_PACKET SOCK_RAW.
--

--- Patch [1]: [PATCH net 1/2] net: packet: fix skb->protocol not updated after VLAN network header adjustment ---
commit f62ea124b860e7404cd95296234dc5505cb676c4
Author: Wei Fang <[email protected]>

net: packet: fix skb->protocol not updated after VLAN network header adjustment

This commit updates skb->protocol with the inner protocol resolved by
vlan_get_protocol_and_depth() for VLAN-tagged frames sent via AF_PACKET
SOCK_RAW. This allows the flow dissector to correctly identify the inner
transport header instead of failing and leaving the transport header unset.

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 0e1355be89f64..a62d445047c0e 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1945,9 +1945,15 @@ static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
>  
>  	/* Move network header to the right position for VLAN tagged packets */
>  	if (likely(skb->dev->type == ARPHRD_ETHER) &&
> -	    eth_type_vlan(skb->protocol) &&
> -	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
> -		skb_set_network_header(skb, depth);
> +	    eth_type_vlan(skb->protocol)) {
> +		__be16 proto = vlan_get_protocol_and_depth(skb, skb->protocol,
> +							   &depth);
> +
> +		if (proto != 0) {
> +			skb_set_network_header(skb, depth);
> +			skb->protocol = proto;

[Severity: High]
Does overwriting skb->protocol here hide the inline VLAN tag from the network
stack and drivers?

Since the VLAN tag is left inline in the MAC payload, updating skb->protocol
to the inner protocol (e.g. ETH_P_IP) causes eth_type_vlan(skb->protocol) to
become false.

This can lead to vlan_features_check() being bypassed, which might allow the
stack to send VLAN packets to a driver with TSO enabled even if the hardware
doesn't support it.

Additionally, drivers like ixgbe that check for ETH_P_8021Q might fail to
recognize the inline software VLAN tag, potentially causing issues like
inserting duplicate VLAN tags when DCB is enabled or breaking egress TC filters.

> +		}
> +	}
>  
>  	skb_probe_transport_header(skb);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1