Re: [PATCH] can: peak_usb: Add bus error reporting for the PCAN-USB FD family

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: peak_usb: NULL pointer dereference in pcan_usb_fd_decode_error()
--

commit 19614d59ec182f3f690df8a2b9505f478b56815e
Author: Stéphane Grosjean <s.grosjean@peak-system.fr>

can: peak_usb: Add bus error reporting for the PCAN-USB FD family

This commit adds CAN bus error reporting for PEAK-System USB-to-CAN FD
devices by enabling firmware bus error notifications per channel.

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> index 0d46f4ce5dca7..fdd38908b4c37 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> @@ -661,6 +661,9 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
>  	struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
>  	struct pcan_usb_fd_device *pdev;
>  	struct peak_usb_device *dev;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u8 can_err_tx = 0;
>  
>  	if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
>  		return -EINVAL;

[Severity: High]
Does this bounds check adequately protect against accessing an uninitialized
channel? Since ARRAY_SIZE(usb_if->dev) is statically 2, if an untrusted
single-channel adapter sends a PUCAN_MSG_ERROR message with channel index 1,
the bounds check passes but the fetched usb_if->dev[1] pointer could be NULL.

> @@ -672,6 +675,53 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
>  	pdev->bec.txerr = er->tx_err_cnt;
>  	pdev->bec.rxerr = er->rx_err_cnt;
>  
> +	/* ignore non-CAN error notifications */
> +	if (PUCAN_ERMSG_TYPE(er) > PUCAN_ERMSG_OTHER_ERROR)
> +		return 0;
> +
> +	/* update other errors counters */
> +	dev->can.can_stats.bus_error++;

[Severity: High]
If the fetched dev pointer is NULL as described above, will this addition
cause a kernel panic when dereferencing dev? Could a maliciously crafted USB
peripheral exploit this to cause a denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-peak_usb-canfd-berr-v1-1-7abd585f877f@peak-system.fr?part=1