Re: [PATCH v7 net-next 09/15] net: dsa: add NETC switch tag support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `netc_flow_dissect` function accesses `skb->data` out-of-bounds without verifying the SKB's linear length.
--

commit 93fb7c6739d18313bc6426f6ff8df23c472a2795
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: add NETC switch tag support

This commit adds support for the NXP NETC switch tag, a proprietary header
added to frames after the source MAC address. It implements the Forward tag,
SubType 0 of the To_Port tag, and Subtype 0 of the To_Host tag.

> diff --git a/net/dsa/tag_netc.c b/net/dsa/tag_netc.c
> new file mode 100644
> index 0000000000000..07684e0ff0647
> --- /dev/null
> +++ b/net/dsa/tag_netc.c

[ ... ]

> +static void netc_flow_dissect(const struct sk_buff *skb, __be16 *proto,
> +			      int *offset)
> +{
> +	struct netc_tag_cmn *tag_cmn = (struct netc_tag_cmn *)(skb->data - 2);
> +	int subtype = FIELD_GET(NETC_TAG_SUBTYPE, tag_cmn->type);

Is it safe to access skb->data directly here without validating the linear
length of the skb?

While the comment below notes that hardware-received frames will have
sufficient linear data, skb_flow_dissect() can also be invoked on locally
injected packets (for example, via AF_PACKET sockets, bpf_prog_test_run,
or tc hooks). For these locally generated packets, skb_headlen(skb) might
be extremely small or even zero.

Could this lead to an out-of-bounds read and kernel panic if the skb is
fragmented? Would it be better to use skb_header_pointer() or validate that
skb_headlen(skb) is large enough before dereferencing?

> +	int type = FIELD_GET(NETC_TAG_TYPE, tag_cmn->type);
> +	int tag_len = netc_get_rx_tag_len(type, subtype);
> +
> +	/* The RX minimum frame length of the NETC switch port is 64 bytes,
> +	 * and the frame is received by the ENETC driver. From the hardware
> +	 * perspective, the receive buffer of RX BD is at least 128 bytes,
> +	 * so the switch tag header is guaranteed to be in the linear region
> +	 * of the skb.
> +	 */
> +	*offset = tag_len;
> +	*proto = ((__be16 *)skb->data)[(tag_len / 2) - 1];
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513030454.1666570-1-wei.fang@nxp.com?part=9