Re: [PATCH net 07/20] can: ctucanfd: handle bus error interrupts

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

Pre-existing issues:
- [Critical] Missing bounds check on hardware-provided word count can lead =
to a severe heap buffer overflow.
- [High] Missing rate-limiting on bus error log messages causes a kernel lo=
g flood and system lockup.
--

--- Patch [7]: [PATCH net 07/20] can: ctucanfd: handle bus error interrupts=
 ---
commit d64540d1abcd187254c2c60ea1d9abba7c60b2be
Author: Avi Weiss <[email protected]>

can: ctucanfd: handle bus error interrupts

Include REG_INT_STAT_BEI in the top-level error interrupt condition. BEI is
enabled when CAN_CTRLMODE_BERR_REPORTING is requested and
ctucan_err_interrupt() already handles it. Without checking and clearing
BEI, bus error interrupts are not handled or acknowledged.

Link: https://patch.msgid.link/[email protected]

> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/c=
tucanfd/ctucanfd_base.c
> index 8f8b1c097ec6d..10ebcc13ea656 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -1136,8 +1136,12 @@ static irqreturn_t ctucan_interrupt(int irq, void =
*dev_id)
>  		/* Error interrupts */
>  		if (FIELD_GET(REG_INT_STAT_EWLI, isr) ||
>  		    FIELD_GET(REG_INT_STAT_FCSI, isr) ||
> -		    FIELD_GET(REG_INT_STAT_ALI, isr)) {
> -			icr =3D isr & (REG_INT_STAT_EWLI | REG_INT_STAT_FCSI | REG_INT_STAT_A=
LI);
> +		    FIELD_GET(REG_INT_STAT_ALI, isr) ||
> +		    FIELD_GET(REG_INT_STAT_BEI, isr)) {
> +			icr =3D isr & (REG_INT_STAT_EWLI |
> +				     REG_INT_STAT_FCSI |
> +				     REG_INT_STAT_ALI |
> +				     REG_INT_STAT_BEI);
> =20
>  			ctucan_netdev_dbg(ndev, "some ERR interrupt: clearing 0x%08x\n", icr);
>  			ctucan_write32(priv, CTUCANFD_INT_STAT, icr);

[Severity: High]
This isn't a bug introduced by this patch, but this patch explicitly unmasks
and clears the BEI interrupt in the top-level handler. Does this prevent the
driver from hitting the previous stuck-interrupt failsafe and inadvertently
unleash an infinite log storm whenever BERR reporting is enabled?

Looking at ctucan_err_interrupt(), the unconditional bus error logging
bypasses the dologerr rate-limit check:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_err_interrupt() {
    ...
    if (FIELD_GET(REG_INT_STAT_BEI, isr)) {
        netdev_info(ndev, "bus error\n");
    ...
}

If the CAN bus is faulty or experiencing heavy interference, will the
controller continuously generate Bus Error Interrupts and overwhelm the
printk subsystem causing a CPU lockup?


[Severity: Critical]
This is a pre-existing issue in the same file, but could the lack of bounds
checking on the hardware-provided word count lead to a severe heap buffer
overflow?

In ctucan_read_rx_frame(), the driver derives the word count directly from
hardware:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_read_rx_frame() {
    ...
    wc =3D FIELD_GET(REG_FRAME_FORMAT_W_RWCNT, ffw) - 3;
    ...
}

If a buggy or malicious hardware controller provides an unexpected word
count, could this integer calculation underflow or result in a large value
that exceeds the maximum 64-byte payload limit?

Later in the same function, the driver copies data into the cf->data
buffer using the unchecked len variable:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_read_rx_frame() {
    ...
    for (i =3D 0; i < len; i +=3D 4) {
        u32 data =3D ctucan_read32(priv, CTUCANFD_RX_DATA);
        *(__le32 *)(cf->data + i) =3D cpu_to_le32(data);
    }
    ...
}

Would this blindly write out of bounds into the heap-allocated SKB buffer?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260729102802.5051=
[email protected]?part=3D7