Re: [PATCH v2 net 1/2] net: packet: fix wrong transport_header when sending VLAN-tagged frame

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

wei.fang@ wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> In packet_parse_headers(), when processing a VLAN-tagged frame,
> skb_set_network_header() is called to advance network_header past the
> VLAN tag to the inner protocol header. skb_probe_transport_header() is
> then called with skb->protocol still set to the outer VLAN EtherType
> (e.g. ETH_P_8021Q), while nhoff (derived from skb_network_offset())
> already points past the VLAN tag to the inner protocol header.
> 
> In __skb_flow_dissect(), proto is initialized to ETH_P_8021Q and nhoff
> points past the VLAN tag. When the dissector hits case ETH_P_8021Q, it
> reads a struct vlan_hdr at nhoff via __skb_header_pointer(), but that
> offset contains the inner protocol header (e.g. an IP header). The bytes
> are misinterpreted as a VLAN header, yielding a garbage encapsulated
> EtherType that matches no known protocol. The dissector returns false,
> so skb_probe_transport_header() never calls skb_set_transport_header(),
> leaving transport_header at its uninitialized sentinel value (~0U).

Not for this patch, but: having a mismatched skb->protocol and
network header offset is asking for trouble anyway.

The blamed commit states that this block is here for GSO packets, and
the mismatch will by fixed up when skb->protocol is updated in
virtio_net_hdr_set_proto. But that only updates GSO packets, while
this block affects all VLAN tagged packets. Both GSO and non-GSO
should be treated similarly throughout the stack. One approach is
setting skb->protocol here. But that does mess with anything matching
VLAN packets. There does not seem to be a perfect solution..

> Move skb_probe_transport_header() to before skb_set_network_header(). At
> the time skb_probe_transport_header() is called, network_header still
> points to the VLAN header, so nhoff correctly points to the VLAN header.
> The flow dissector can then parse the VLAN header, extract the inner
> EtherType, and advance nhoff to the inner protocol header, allowing
> transport_header to be set correctly.
> 
> Fixes: dfed913e8b55 ("net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO")
> Assisted-by: WChat:claude-opus-4-8
> Signed-off-by: Wei Fang <wei.fang@nxp.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>