Re: [PATCH net-next v14 1/9] net: dsa: add tag driver for LAN9645X

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net/dsa: lan9645x: Stale skb->mac_header and skb->network_header after vlan_remove_tag()
--

commit 2bf43370c466c901725bcb460aaa53654797d172
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
Subject: net: dsa: add tag driver for LAN9645X

This commit adds a tag driver for the LAN9645x family of switches, using a
front port configured as an NPI port for CPU traffic. It handles frame
extraction and injection using internal frame headers and prefix formatting.

Link: None provided

> diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c
> new file mode 100644
> index 0000000000000..00339daecfcbd
> --- /dev/null
> +++ b/net/dsa/tag_lan9645x.c
[ ... ]
> +static struct sk_buff *lan9645x_xmit_get_vlan_info(struct sk_buff *skb,
> +						   struct net_device *br,
> +						   u32 *vlan_tci,
> +						   u32 *tag_type)
> +{
[ ... ]
> +		hdr = skb_vlan_eth_hdr(skb);
> +		if (ntohs(hdr->h_vlan_proto) == proto &&
> +		    ntohs(hdr->h_vlan_TCI) & VLAN_VID_MASK) {
> +			vlan_remove_tag(skb, &tci);
> +			*vlan_tci = tci;

[Severity: Medium]
When calling vlan_remove_tag() here, are the skb mac_header and
network_header metadata offsets left pointing to stale locations?

The vlan_remove_tag() function removes the vlan tag and moves the payload,
but it expects the caller to update skb->mac_header and skb->network_header.
This is why the standard helper __skb_vlan_pop() is usually preferred.

Because this tag driver does not adjust skb->mac_header, the offset becomes
stale. After lan9645x_xmit() pushes the 28-byte internal frame header,
won't skb_mac_header(skb) incorrectly point to bytes inside the new header
rather than the actual start of the destination address?

If the conduit device driver or network stack relies on skb_mac_header(skb)
for transmit processing, this could cause it to read incorrect bytes for
packets that had a vlan tag stripped.

> +		}
> +	}
> +
> +	*tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S :
> +					     LAN9645X_IFH_TAG_TYPE_C;
> +
> +	return skb;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-dsa_lan9645x_switch_driver_base-v14-0-3d2a72661213@microchip.com?part=1