Re: [PATCH] can: sja1000: drop RX frame when skb allocation fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In sja1000_err(), an SKB allocation failure returns -ENOMEM, discarding IRQ_WAKE_THREAD requests and causing the interrupt handler to erroneously return IRQ_NONE.
--

commit 6507d559134b8295113fac43d6ce819e79885772
Author: Yun Lu <luyun@kylinos.cn>

can: sja1000: drop RX frame when skb allocation fails

This patch prevents a lockup in sja1000_rx() by dropping the frame and
releasing the receive buffer if SKB allocation fails.

> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index 3cdb583ee3e50..da107feb22af7 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -348,8 +348,12 @@ static void sja1000_rx(struct net_device *dev)
>  
>  	/* create zero'ed CAN frame buffer */
>  	skb = alloc_can_skb(dev, &cf);
> -	if (skb == NULL)
> +	if (!skb) {
> +		/* drop the frame to guarantee forward progress of the ISR */
> +		sja1000_write_cmdreg(priv, CMD_RRB);
> +		stats->rx_dropped++;
>  		return;
> +	}
>  
>  	fi = priv->read_reg(priv, SJA1000_FI);

[Severity: High]
This is a pre-existing issue, but while looking at SKB allocation failures
in the interrupt handler, does a similar allocation failure in
sja1000_err() lead to the IRQ line being permanently disabled?

Under memory pressure, if the SKB allocation fails in sja1000_err():

drivers/net/can/sja1000/sja1000.c:sja1000_err() {
    ...
	if (!skb)
		return -ENOMEM;

	netif_rx(skb);

	return ret;
}

This unconditionally returns -ENOMEM, which discards any IRQ_WAKE_THREAD
requests needed for overrun recovery like SJA1000_QUIRK_RESET_ON_OVERRUN.

This error propagates to sja1000_interrupt(), causing the loop to break
before incrementing n:

drivers/net/can/sja1000/sja1000.c:sja1000_interrupt() {
    ...
			err = sja1000_err(dev, isrc, status);
			if (err == IRQ_WAKE_THREAD)
				ret = err;
			if (err)
				break;
		}
		n++;
    ...
}

If this happens on the first loop iteration where n is 0, the handler
returns IRQ_NONE despite having successfully acknowledged the interrupt in
hardware:

drivers/net/can/sja1000/sja1000.c:sja1000_interrupt() {
    ...
out:
	if (!ret)
		ret = (n) ? IRQ_HANDLED : IRQ_NONE;
    ...
}

Could repeated IRQ_NONE returns under sustained memory pressure cause the
core IRQ subsystem to permanently disable the device's IRQ line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903095444.495807-1-luyun_611@163.com?part=1