Re: [PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path

[eritque-arcus@xxxxxxxxx]

Hi Marc, Vincent,

Please drop patch 2/2 (kvaser_usb_hydra) from this series. Cen Zhang has 
since posted a standalone fix for the same hydra receive path that is 
more complete than mine.

<https://lore.kernel.org/linux-can/20260819145658.29872-1-blbllhy@xxxxxxxxx/>

Patch 1/2 (esd_usb) is independent so please consider it on its own.


One note on the automated review of patch 1/2: the issues it raised are 
all in esd_usb's probe and tx-done paths. I've kept this fix narrow 
rather than widen it into those, and will look at them separately.


Thanks,
Yiran

On 8/14/26 2:05 PM, Yiran Qiu wrote:
> kvaser_usb_hydra_read_bulk_callback() walks commands out of the RX URB
> buffer, using kvaser_usb_hydra_cmd_size() to determine each command's
> length. For an extended command (CMD_EXTENDED) that size is taken
> directly from the device-supplied 16-bit length field with no lower
> bound. A CMD_EXTENDED command whose length is zero makes cmd_size 0, so
> "pos += cmd_len" never advances and this URB-completion softirq spins
> forever.
>
> Reject a command whose reported size is smaller than the command header
> before it is dispatched, mirroring the minimum-length check added in
> commit 0293dd153f9d ("can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd():
> validate received command extents"); that fix did not touch hydra's
> asynchronous read_bulk_callback().
>
> Reproduced with USB_RAW_GADGET + dummy_hcd on a KASAN build: after the
> normal probe/START_CHIP handshake, a 6-byte CMD_EXTENDED frame with the
> length field set to 0 makes the callback loop print
>
>    kvaser_usb 1-1:1.0: Unhandled extended command (255)
>
> without bound (306000 times in ~75 s), until
>
>    rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
>
> and the machine had to be killed externally.
>
> Fixes: aec5fb2268b5 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Yiran Qiu <eritque-arcus@xxxxxxxxx>
> ---
>   drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> index efbb7bed34c9d..d44f9875fbe2f 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> @@ -2156,6 +2156,15 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
>   
>   		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
>   
> +		/* An extended command carries a device-supplied length; a
> +		 * command shorter than the command header would never advance
> +		 * @pos and would spin this URB-completion softirq forever.
> +		 */
> +		if (cmd_len < sizeof(struct kvaser_cmd_header)) {
> +			dev_err(&dev->intf->dev, "Format error\n");
> +			break;
> +		}
> +
>   		if (pos + cmd_len > len) {
>   			/* We got first part of a command */
>   			int leftover_bytes;