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

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 e=
ndpoint with a zero wMaxPacketSize causes an infinite loop and CPU soft loc=
kup.
--

--- Patch [14]: [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wai=
t_cmd(): validate received command extents ---
commit ad6fd8163961ebd7ee69b192c06c6c10803d720a
Author: Pengpeng Hou <[email protected]>
Subject: can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate receive=
d 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/[email protected]

> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/n=
et/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 kv=
aser_usb *dev, u8 id,
>  				continue;
>  			}
> =20
> -			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 abo=
ve
this bounds check still allow an infinite loop?

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

			if (tmp->len =3D=3D 0) {
				pos =3D 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 fai=
l 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 l=
ength
of 0, kvaser_usb_hydra_cmd_size() returns 0. The loop then increments pos b=
y 0,
failing to advance and causing a soft lockup.

[ ... ]

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