Re: [PATCH net-next v4 5/8] net: dsa: Add tag handling for SoC-e switches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net: dsa: insufficient skb headroom in sdsa_xmit() for
  non-802.1Q VLAN
--

commit ae5577cfb919384ffe32e9238c7345ef431b5a12
Author: Vasilij Strassheim <v.strassheim@linutronix.de>

net: dsa: Add tag handling for SoC-e switches

This commit adds a DSA tag driver for SoC-e switches using an 8-byte
EtherType 0xdcdc SDSA header. It handles transmit and receive frame
tagging, including 802.1Q TCI metadata encoding and non-802.1Q VLAN
header insertion.

> diff --git a/net/dsa/tag_sdsa.c b/net/dsa/tag_sdsa.c
> new file mode 100644
> index 0000000000000..dcfa5de0ac6bd
> --- /dev/null
> +++ b/net/dsa/tag_sdsa.c
> @@ -0,0 +1,168 @@

[ ... ]

> +static struct sk_buff *sdsa_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_user_to_port(dev);
> +	struct sdsa_tag *tag;
> +	bool encode_vlan;
> +	bool tag_present;
> +	u16 vlan_tci;
> +
> +	BUILD_BUG_ON(sizeof(*tag) != SDSA_HLEN);
> +
> +	/* SDSA carries no TPID, so only encode 802.1Q C-tags. */
> +	tag_present = skb_vlan_tag_present(skb);
> +	encode_vlan = tag_present && skb->vlan_proto == htons(ETH_P_8021Q);
> +	/* Prevent the conduit from inserting a VLAN header ahead of SDSA. */
> +	if (tag_present && !encode_vlan) {
> +		skb = __vlan_hwaccel_push_inside(skb);
> +		if (!skb)
> +			return NULL;
> +	}
> +
> +	skb_push(skb, SDSA_HLEN);

[Severity: High]
Can this skb_push(skb, SDSA_HLEN) underflow skb->head and trigger
skb_under_panic() when transmitting a packet with a non-802.1Q hardware
accelerated VLAN tag (for example ETH_P_8021AD)?

sdsa_netdev_ops sets needed_headroom to SDSA_HLEN (8 bytes), so
dsa_user_xmit() -> skb_ensure_writable_head_tail() only guarantees
SDSA_HLEN + conduit->needed_headroom bytes of headroom before calling
sdsa_xmit():

dsa_user_xmit()
  skb_ensure_writable_head_tail(skb, dev)
  sdsa_xmit()
    __vlan_hwaccel_push_inside()
      __vlan_insert_inner_tag()
        skb_cow_head(skb, VLAN_HLEN)
        skb_push(skb, VLAN_HLEN)
    skb_push(skb, SDSA_HLEN)

When skb enters sdsa_xmit() with 8 to 11 bytes of headroom,
__vlan_hwaccel_push_inside() sees enough room for VLAN_HLEN (4 bytes)
without reallocating and consumes 4 bytes via skb_push(skb, VLAN_HLEN),
leaving only 4 to 7 bytes of headroom before skb_push(skb, SDSA_HLEN)
pushes another 8 bytes.

Should sdsa_xmit() call skb_cow_head() before pushing, or should
sdsa_netdev_ops reserve SDSA_HLEN + VLAN_HLEN in needed_headroom?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-devel-vstrassheim-soce-dsa-ml-v4-0-02f3cde2afa1@linutronix.de?part=5