Re: [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Processing a zero-length extended command or encountering a bulk endpoint with a zero wMaxPacketSize causes an infinite loop and CPU soft lockup.
--

--- Patch [14]: [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents ---
commit ad6fd8163961ebd7ee69b192c06c6c10803d720a
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
Subject: can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents

The wait and bulk receive paths walk variable-length commands from a
USB buffer. A nonzero command shorter than CMD_HEADER_LEN can still be
dispatched, and the wait path copies a matching command into a fixed
caller-owned struct kvaser_cmd using the device-provided length.

Link: https://patch.msgid.link/20260722042221.44066-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> index df737cfc5ea0c..a876c7819b81c 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -691,13 +691,22 @@ static int kvaser_usb_leaf_wait_cmd(const struct kvaser_usb *dev, u8 id,
>  				continue;
>  			}
>  
> -			if (pos + tmp->len > actual_len) {
> +			if (tmp->len < CMD_HEADER_LEN ||
> +			    tmp->len > actual_len - pos) {

[Severity: High]
This isn't a bug introduced by this patch, but might the logic directly above
this bounds check still allow an infinite loop?

If a USB device provides a bulk IN endpoint with wMaxPacketSize == 0, the check
for tmp->len == 0 behaves as follows:

			if (tmp->len == 0) {
				pos = round_up(pos + 1,
					       le16_to_cpu
						(dev->bulk_in->wMaxPacketSize));
				continue;
			}

Here, round_up(pos + 1, 0) evaluates to 0, which resets pos to 0. Since this
executes before the new tmp->len < CMD_HEADER_LEN check, the loop would fail to
advance and hang the process.

This same pattern appears in kvaser_usb_leaf_read_bulk_callback() modified
later in this patch.

Additionally, a similar pre-existing issue seems to be present in the
kvaser_usb_hydra driver variant. In kvaser_usb_hydra_wait_cmd() and
kvaser_usb_hydra_read_bulk_callback(), if an extended command specifies a length
of 0, kvaser_usb_hydra_cmd_size() returns 0. The loop then increments pos by 0,
failing to advance and causing a soft lockup.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=14