Re: [PATCH net-next v4 2/3] net: ti: icssm-prueth: Add priority based RX IRQ handlers

[Simon Horman <horms@kernel.org>]

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://sashiko.dev/#/patchset/20260921134343.200426-4-parvathi@couthit.com?part=2
---
Subject: net: ti: icssm-prueth: Add priority based RX IRQ handlers

This commit adds support for priority-based interrupt handling for the STP/RSTP
Switch, HSR, and PRP protocols to address port dominance. It records an IEP
hardware timestamp into the receive buffer to guarantee frames are delivered in
wire-arrival order. The patch also converts statistics counters to atomic64_t
and adds a firmware revision field to support different timestamp wraparound
behaviors.

> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
> index 6c8888d9d66a6..4a37935b8d352 100644
> --- a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
> +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
[ ... ]
> @@ -1071,11 +1083,25 @@ static int icssm_emac_ndo_open(struct net_device *ndev)
>  			goto iep_exit;
>  	}
>  
> -	ret = icssm_emac_request_irqs(emac);
> -	if (ret)
> -		goto rproc_shutdown;
> +	if (PRUETH_IS_EMAC(prueth)) {
> +		napi_enable(&emac->napi);
> +	} else {
> +		if (!prueth->emac_configured &&
> +		    (PRUETH_IS_SWITCH(prueth) || prueth_is_lre(prueth))) {
> +			napi_enable(&prueth->napi_hpq);
> +			napi_enable(&prueth->napi_lpq);

[Severity: High]
Does this code risk a NULL pointer dereference?

If MII0 is missing (e.g., absent from the device tree), the shared NAPI
structures napi_hpq and napi_lpq are not initialized in
icssm_prueth_netdev_init(). If the MII1 interface is later placed into SWITCH
mode and brought up, it appears icssm_emac_ndo_open() will unconditionally call
napi_enable() on these zero-initialized structures, causing a NULL pointer
dereference on n->dev.

> +		}
> +	}
[ ... ]
> @@ -1780,9 +1820,25 @@ static int icssm_prueth_netdev_init(struct prueth *prueth,
>  
>  	netif_napi_add(ndev, &emac->napi, icssm_emac_napi_poll);
>  
> +	if ((prueth->support_lre || fw_data->support_switch) &&
> +	    emac->port_id == PRUETH_PORT_MII0) {
> +		netif_napi_add(ndev, &prueth->napi_hpq,
> +			       icssm_prueth_common_napi_poll_hpq);
> +		netif_napi_add(ndev, &prueth->napi_lpq,
> +			       icssm_prueth_common_napi_poll_lpq);
> +	}
> +
>  	hrtimer_setup(&emac->tx_hrtimer, &icssm_emac_tx_timer_callback,
>  		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
>  
> +	if ((prueth->support_lre || fw_data->support_switch) &&
> +	    emac->port_id == PRUETH_PORT_MII0) {
> +		prueth->hp->ndev = ndev;
> +		prueth->hp->priority = 0;
> +		prueth->lp->ndev = ndev;
> +		prueth->lp->priority = 1;
> +	}
> +
>  	return 0;
[ ... ]
> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c b/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c
> new file mode 100644
> index 0000000000000..9b7b876240b5f
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c
[ ... ]
> +static irqreturn_t icssm_prueth_common_emac_rx_hardirq(int irq, void *dev_id)
> +{
> +	struct prueth_ndev_priority *ndev_prio;
> +	struct prueth_emac *emac;
> +	struct net_device *ndev;
> +	struct prueth *prueth;
> +
> +	ndev_prio = (struct prueth_ndev_priority *)dev_id;
> +	ndev = ndev_prio->ndev;

[Severity: High]
Similarly, can this IRQ handler dereference an uninitialized pointer?

If MII0 is missing and these priority structures were skipped during
initialization, it looks like the registered IRQ handler will dereference the
zero-initialized ndev_prio->ndev here when an interrupt occurs.

> +	emac = netdev_priv(ndev);
> +	prueth = emac->prueth;

[Severity: Medium]
This is a pre-existing issue, but can the transmission path read out of bounds?

In drivers/net/ethernet/ti/icssm/icssm_prueth.c:icssm_prueth_get_tx_queue_id():

    err = vlan_get_tag(skb, &vlan_tci);

icssm_emac_ndo_start_xmit() calls icssm_prueth_get_tx_queue_id() before
validating the length of the transmitted skb or padding it. Because the device
lacks NETIF_F_HW_VLAN_CTAG_TX, vlan_get_tag() falls back to __vlan_get_tag(),
which reads the VLAN TCI at offset 14. If a raw socket transmits a packet
smaller than 18 bytes (e.g., exactly 14 bytes) with h_vlan_proto ==
ETH_P_8021Q, this reads unallocated memory past the buffer.