Re: [PATCH net] dpaa2-switch: add bounds check for if_id in IRQ handler

[Guenter Roeck <linux@roeck-us.net>]

Hi,

On Thu, Jan 29, 2026 at 12:55:13AM +0800, Junrui Luo wrote:
> The IRQ handler extracts if_id from the upper 16 bits of the hardware
> status register and uses it to index into ethsw->ports[] without
> validation. Since if_id can be any 16-bit value (0-65535) but the ports
> array is only allocated with sw_attr.num_ifs elements, this can lead to
> an out-of-bounds read potentially.
> 
> Add a bounds check before accessing the array, consistent with the
> existing validation in dpaa2_switch_rx().
> 
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Fixes: 24ab724f8a46 ("dpaa2-switch: use the port index in the IRQ handler")
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index b1e1ad9e4b48..33f0842b5dc9 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> @@ -1531,6 +1531,10 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
>  	}
>  
>  	if_id = (status & 0xFFFF0000) >> 16;
> +	if (if_id >= ethsw->sw_attr.num_ifs) {
> +		dev_err(dev, "Invalid if_id %d in IRQ status\n", if_id);
> +		goto out;
> +	}

An experimental AI code review agent produced the following feedback:

Will jumping to the out label here cause an interrupt storm?

It looks like this bypasses the dpsw_clear_irq_status() call at the end
of the function. If the hardware interrupt status isn't cleared, it might
leave the interrupt asserted and cause the handler to trigger continuously.
Should this code clear the status before returning?

It seems to me that it has a point, and that the code should at least attempt
to reset the interrupt status. Please let me know if this is correct or
if this is not a concern.

Thanks,
Guenter

>  	port_priv = ethsw->ports[if_id];
>  
>  	if (status & DPSW_IRQ_EVENT_LINK_CHANGED)
> 
> ---
> base-commit: a040afa3bca415019d96a586b96b5f17b1f55a90
> change-id: 20260129-fixes-98a0f7607a88
> 
> Best regards,
> -- 
> Junrui Luo <moonafterrain@outlook.com>
>