Re: [PATCH] can: peak_usb: pcan_usb_fd: check that the channel has a device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] can: peak_usb: missing PCAN_USB_STATE_STARTED check in
  pcan_usb_fd_decode_buf() [1]
--

commit b1428d71020b5f4ac8a4435284c960096e6c1169
Author: Joas Antonio dos Santos <joasantonio108@gmail.com>

can: peak_usb: pcan_usb_fd: check that the channel has a device

This commit checks that the per-channel device pointer looked up in
usb_if->dev[] is non-NULL in the four pcan_usb_fd RX decoders before
dereferencing it.

> 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 39f0740421c36..c42cac2126404 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c

[ ... ]

> @@ -609,6 +612,9 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
>  		return -ENOMEM;
>  
>  	dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
> +	if (!dev)
> +		return -ENODEV;
> +
>  	pdev = container_of(dev, struct pcan_usb_fd_device, dev);
>  	netdev = dev->netdev;

[Severity: Critical]
This problem wasn't introduced by this patch, but can these decoders still
process messages for a stopped channel [1] when a sibling channel on
the same adapter remains open?

On multi-channel adapters that share a bulk IN endpoint,
peak_usb_read_bulk_callback() checks PCAN_USB_STATE_STARTED only on
urb->context before calling pcan_usb_fd_decode_buf().  None of the four
decoders check whether the target channel looked up in usb_if->dev[] has
PCAN_USB_STATE_STARTED set.

If a PUCAN_BUS_BUSOFF status message is decoded for a channel that has
already been closed via peak_usb_ndo_stop(), pcan_usb_fd_decode_status()
calls can_bus_off() and re-arms dev->can.restart_work after close_candev()
canceled it.  That work can then restart the closed controller, or cause a
use-after-free if the USB device is disconnected and freed before the
delayed work runs.

[1] https://sashiko.dev/#/bug/linux-a0866161-9660-4bc4-9535-9260486564d9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-pcan-usb-fd-nodev-v1-1-fc11effbfafa@gmail.com?part=1