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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Out-of-bounds read and potential kernel panic in `netc_flow_dissect` due to direct pointer dereferencing without bounds or linearity checks.
- [Low] Missing explicit inclusion of `<linux/bitfield.h>` in `net/dsa/tag_netc.c`.
--

commit 9e0475f22527eda82a3169b05fab7628a34dfd3e
Author: Wei Fang <wei.fang@nxp.com>
Subject: net: dsa: add NETC switch tag support

This commit introduces support for the proprietary NXP NETC switch tag
header, which is added to frames after the source MAC address. It supports
Forward tags and SubType 0 of both To_Port and To_Host tags.

> diff --git a/net/dsa/tag_netc.c b/net/dsa/tag_netc.c
> new file mode 100644
> index 0000000000000..ccedfe3a80b64
> --- /dev/null
> +++ b/net/dsa/tag_netc.c
> @@ -0,0 +1,214 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025-2026 NXP
> + */
> +
> +#include <linux/dsa/tag_netc.h>
> +
> +#include "tag.h"

This isn't a bug, but should linux/bitfield.h be explicitly included here?
This file uses the FIELD_GET and FIELD_PREP macros.

[ ... ]

> +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);
> +	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];

Can this cause an out-of-bounds read if an skb is injected from userspace
(e.g., via raw sockets, traffic control filters, or BPF) directed at the
conduit interface?

If the injected skb has zero headroom (skb->data == skb->head), reading
skb->data - 2 will underflow the allocated buffer.

Similarly, if the injected skb is fragmented and lacks sufficient data in
the linear region, reading the inner protocol directly from skb->data
could read past the valid linear area.

Since the skb is passed as const to flow_dissect, pskb_may_pull cannot
be used. Would it be better to use skb_header_pointer to safely access
these packet headers?

> +}

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