* Re: [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG
2026-08-07 11:17 [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG Siddharth Vadapalli
@ 2026-08-09 20:27 ` Chintan Vankar
2026-08-10 16:56 ` Simon Horman
1 sibling, 0 replies; 3+ messages in thread
From: Chintan Vankar @ 2026-08-09 20:27 UTC (permalink / raw)
To: Siddharth Vadapalli, andrew+netdev, davem, edumazet, kuba, pabeni,
horms, vadim.fedorenko, v-singh1, nm, daniel.zahka, nora.schiffer,
oss, grygorii.strashko
Cc: netdev, linux-kernel, linux-arm-kernel, srk
On 07/08/26 16:47, Siddharth Vadapalli wrote:
> On the packet reception path, the ID of the MAC Port on which the packet
> was received, is embedded in the RX DMA Descriptor's metadata. The ID is
> extracted using the helper function cppi5_desc_get_tags_ids() which fills
> in the 16-bit Source Tag into the 'port_id' variable. However, it is only
> the lower 8-bits of the 16-bit Source Tag that represent the MAC Port ID,
> while the upper 8-bits are Hardware-Reserved and carry an arbitrary value.
> With the existing logic, sporadic kernel crash is observed due to the
> subsequent driver code accessing out-of-bound memory because of an invalid
> port_id.
>
> Hence, fix the port_id extraction logic to use only the lower 8-bits of the
> Source Tag as the MAC Port ID.
>
> Fixes: 93a76530316a ("net: ethernet: ti: introduce am65x/j721e gigabit eth subsystem driver")
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>
> Hello,
>
> This patch is based on commit
> f9a2394a2348 Merge tag 'mm-hotfixes-stable-2026-08-06-18-44' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> of Mainline Linux.
>
> I have observed it in practice that the RESERVED field contains non-zero
> value (such as 0xFF on one instance) causing port_id to be set to an
> invalid value (such as 0xFF01 in that instance) resulting in the subsequent
> driver code accessing out-of-bound / incorrect memory and corrupting it and
> causing a kernel crash. To summarize, this patch is fixing a real issue and
> not a hypothetical one.
>
> Regards,
> Siddharth.
>
> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 434a31080855..6330540979d7 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1310,6 +1310,8 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_rx_flow *flow,
> k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
> pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
> cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
> + /* Port ID is contained in the lower 8-bits of the 16-bit Source Tag */
> + port_id &= 0xFF;
> dev_dbg(dev, "%s rx port_id:%d\n", __func__, port_id);
> port = am65_common_get_port(common, port_id);
> ndev = port->ndev;
Reviewed-by: Chintan Vankar <c-vankar@ti.com>
Regards,
Chintan.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG
2026-08-07 11:17 [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG Siddharth Vadapalli
2026-08-09 20:27 ` Chintan Vankar
@ 2026-08-10 16:56 ` Simon Horman
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2026-08-10 16:56 UTC (permalink / raw)
To: s-vadapalli
Cc: Simon Horman, andrew+netdev, davem, edumazet, kuba, pabeni,
c-vankar, vadim.fedorenko, v-singh1, nm, daniel.zahka,
nora.schiffer, oss, grygorii.strashko, netdev, linux-kernel,
linux-arm-kernel, srk
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260807111738.2055900-1-s-vadapalli@ti.com?part=1
---
--- Patch [1]: [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG ---
commit b0ed959d350c90d8b49f6f81eba7461fbc86dcf4
Author: Siddharth Vadapalli <s-vadapalli@ti.com>
net: ethernet: ti: am65-cpsw-nuss: Fix port_id extraction from SRC TAG
The RX path extracts the ingress MAC port ID from the CPPI5 descriptor's
16-bit Source Tag. Only the lower 8 bits carry the port ID, so the patch
masks the value with 0xFF before it is used as an index into
common->ports[].
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 434a31080855..6330540979d7 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1310,6 +1310,8 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_rx_flow *flow,
> k3_udma_glue_rx_cppi5_to_dma_addr(rx_chn->rx_chn, &buf_dma);
> pkt_len = cppi5_hdesc_get_pktlen(desc_rx);
> cppi5_desc_get_tags_ids(&desc_rx->hdr, &port_id, NULL);
> + /* Port ID is contained in the lower 8-bits of the 16-bit Source Tag */
> + port_id &= 0xFF;
> dev_dbg(dev, "%s rx port_id:%d\n", __func__, port_id);
> port = am65_common_get_port(common, port_id);
> ndev = port->ndev;
[Severity: High]
[Finding: main-1-0]
[Sources: opus-5]
The missing range check on port_id is pre-existing and not introduced by
this patch, but since the change is specifically about avoiding the
out-of-bounds access, would it be worth validating the value instead of
relying only on the mask?
After the mask, port_id is still an unvalidated hardware-supplied value in
the range 0..255, and am65_common_get_port() does raw pointer arithmetic
with no bounds check:
drivers/net/ethernet/ti/am65-cpsw-nuss.h:
#define am65_common_get_port(common, id) (&(common)->ports[(id) - 1])
common->ports is a devm_kcalloc() array of exactly common->port_num
entries (1..AM65_CPSW_MAX_PORTS, checked in am65_cpsw_nuss_probe()), which
is typically 1 or 2 on shipping parts. So port_id == 0 gives ports[-1] and
any port_id above common->port_num reads past the end of the allocation.
The resulting pointer is dereferenced right away in
am65_cpsw_nuss_rx_packets():
port = am65_common_get_port(common, port_id);
ndev = port->ndev;
...
if (port->xdp_prog) {
and ndev then flows into am65_cpsw_build_skb(), netdev_priv(),
netif_dormant() and ndev->stats.*. Can a stray low byte still produce the
same class of crash the commit message describes?
The driver already enforces the required index domain for this accessor on
the device tree path in am65_cpsw_nuss_init_slave_ports():
if (!port_id || port_id > common->port_num) {
dev_err(dev, "%pOF has invalid port_id %u %s\n",
port_np, port_id, port_np->name);
ret = -EINVAL;
goto of_node_put;
}
port = am65_common_get_port(common, port_id);
Would the same !port_id || port_id > common->port_num test in the RX path,
dropping the packet and requeueing the page (bumping rx_errors, perhaps
with a WARN_ON_ONCE), be a more robust fix than the mask alone?
There is a second case even when the index is in range: ports disabled in
the device tree never get an ndev allocated, and every other iteration site
guards for that, for example in am65_cpsw_nuss_set_offload_fwd_mark():
if (!port->ndev)
continue;
The RX path uses port->ndev unconditionally, so an in-range port_id
belonging to a disabled port would hand a NULL net_device to
am65_cpsw_build_skb() and netdev_priv(). Should that case be rejected too?
As a small nit, would a named mask defined alongside the other CPPI5/AM65
definitions read better here than the bare 0xFF literal?
^ permalink raw reply [flat|nested] 3+ messages in thread