Re: [PATCH 3/4] net: dsa: Add support for SoC-e SDSA tags

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `sdsa_xmit` and `sdsa_rcv` functions leak the SKB on all error paths by returning `NULL` without freeing the buffer, leading to memory exhaustion.
--

--- Patch [3]: [PATCH 3/4] net: dsa: Add support for SoC-e SDSA tags ---
commit d07af353633eb9eecc24042cfdf75f9ecf082368
Author: Vasilij Strassheim <v.strassheim@linutronix.de>

net: dsa: Add support for SoC-e SDSA tags

Adds a DSA tagger for the SoC-e SDSA header format used between
the conduit and SoC-e switch ports.

> diff --git a/net/dsa/tag_sdsa.c b/net/dsa/tag_sdsa.c
> new file mode 100644
> --- /dev/null
> +++ b/net/dsa/tag_sdsa.c

[ ... ]

> +static struct sk_buff *sdsa_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_user_to_port(dev);
> +	u8 *sdsa_header;
> +
> +	if (skb_cow_head(skb, SDSA_HLEN) < 0)
> +		return NULL;

[Severity: High]
Does this leak the skb when skb_cow_head() fails? The DSA core expects the
tagger to consume and free the skb if it returns NULL on transmission. Should
this call kfree_skb() before returning?

[ ... ]

> +static struct sk_buff *sdsa_rcv(struct sk_buff *skb, struct net_device *dev)
> +{
> +	u8 *sdsa_header;
> +	int source_port;
> +	u8 frame_type;
> +
> +	if (unlikely(!pskb_may_pull(skb, SDSA_HLEN)))
> +		return NULL;

[Severity: High]
Does this error path leak the skb? If a malformed packet causes
pskb_may_pull() to fail, returning NULL without freeing the buffer could
allow network traffic to cause memory exhaustion.

> +
> +	sdsa_header = dsa_etype_header_pos_rx(skb);
> +
> +	/* Check that the frame type is TO_CPU. */
> +	frame_type = FIELD_GET(SDSA_TAG_FRAME_TYPE_MASK, sdsa_header[4]);
> +	if (frame_type != 0)
> +		return NULL;

[Severity: High]
Should the skb be freed here when the frame type check fails to avoid a
memory leak?

> +
> +	/* VLAN-tagged traffic is intentionally unsupported yet. */
> +	if (sdsa_header[4] & SDSA_TAG_VLAN_BIT)
> +		return NULL;

[Severity: High]
Could unsupported VLAN-tagged traffic trigger a memory leak here?

> +
> +	/* Determine the source port from the two port fields. */
> +	source_port = ((sdsa_header[4] & 0x1f) << 5) |
> +		      ((sdsa_header[5] & 0xf8) >> 3);
> +
> +	skb->dev = dsa_conduit_find_user(dev, 0, source_port);
> +	if (!skb->dev)
> +		return NULL;

[Severity: High]
Does this also leak the skb if the conduit cannot find a corresponding user
device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-devel-vstrassheim-soce-dsa-ml-v1-0-be569dae1b20@linutronix.de?part=3