[PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path

[PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path

[Cen Zhang (Microsoft)]

The receive-path command parsers (kvaser_usb_hydra_wait_cmd and
kvaser_usb_hydra_read_bulk_callback) call kvaser_usb_hydra_cmd_size()
without verifying that enough buffer remains.  For CMD_EXTENDED,
kvaser_usb_hydra_cmd_size() unconditionally reads a 2-byte len field
at offset 4 (kvaser_usb_hydra.c:532).  A malicious USB device can place
a CMD_EXTENDED header at the end of a 3072-byte bulk transfer such that
only 4 bytes remain, causing a 2-byte slab-out-of-bounds read.

  BUG: KASAN: slab-out-of-bounds in kvaser_usb_hydra_wait_cmd+0x3f1/0x480
    [kvaser_usb_hydra.c:678]
  Read of size 2 at addr ffff888013f7ec00 by task kworker/0:0/9
   kvaser_usb_hydra_wait_cmd+0x3f1/0x480
   kvaser_usb_hydra_get_software_details+0x1c7/0x5d0
   kvaser_usb_probe+0x36a/0x1240

Additionally, if the device sends CMD_EXTENDED with len=0,
kvaser_usb_hydra_cmd_size() returns 0 and the parser loops forever
(pos += 0), permanently burning one CPU core.

Fix by adding kvaser_usb_hydra_cmd_size_rx(), a receive-path wrapper that
validates buffer bounds before calling kvaser_usb_hydra_cmd_size().
Callers check for zero return to reject both issues above.  Clear invalid
leftover state before returning so subsequent transfers do not retry the
same malformed command.

Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Link: https://lore.kernel.org/all/20260819145658.29872-1-blbllhy@gmail.com
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
v2:
 - Clear malformed leftover state before returning.
 - Reject command lengths shorter than the buffered prefix.

v1:
 - The zero-length command loop is also addressed by:
    https://lore.kernel.org/linux-can/20260815-can-esd-hydra-fixes-v1-2-de644cbeaec2@ikuyo.dev/
 - This patch additionally handles truncated command headers in the
    synchronous wait and asynchronous receive paths, including the
    leftover-buffer path.

 .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 36 ++++++++++++++++---
 1 file changed, 32 insertions(+), 4 deletions(-)

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 efbb7bed34c9..534bd2683012 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -536,6 +536,20 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
 	return ret;
 }
 
+/* Receive-path wrapper: validate buffer bounds before reading cmd_size. */
+static size_t kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd,
+					   size_t remaining)
+{
+	if (remaining < sizeof(struct kvaser_cmd_header))
+		return 0;
+
+	if (cmd->header.cmd_no == CMD_EXTENDED &&
+	    remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext))
+		return 0;
+
+	return kvaser_usb_hydra_cmd_size(cmd);
+}
+
 static struct kvaser_usb_net_priv *
 kvaser_usb_hydra_net_priv_from_cmd(const struct kvaser_usb *dev,
 				   const struct kvaser_cmd *cmd)
@@ -675,8 +689,9 @@ static int kvaser_usb_hydra_wait_cmd(const struct kvaser_usb *dev, u8 cmd_no,
 			size_t cmd_len;
 
 			tmp_cmd = buf + pos;
-			cmd_len = kvaser_usb_hydra_cmd_size(tmp_cmd);
-			if (pos + cmd_len > actual_len) {
+			cmd_len = kvaser_usb_hydra_cmd_size_rx(tmp_cmd,
+							       actual_len - pos);
+			if (!cmd_len || pos + cmd_len > actual_len) {
 				dev_err_ratelimited(&dev->intf->dev,
 						    "Format error\n");
 				break;
@@ -2124,7 +2139,15 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 
 		cmd = (struct kvaser_cmd *)card_data->usb_rx_leftover;
 
-		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+		cmd_len = kvaser_usb_hydra_cmd_size_rx(cmd,
+						       KVASER_USB_HYDRA_MAX_CMD_LEN);
+
+		if (!cmd_len || cmd_len < usb_rx_leftover_len) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			card_data->usb_rx_leftover_len = 0;
+			spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
+			return;
+		}
 
 		remaining_bytes = min_t(unsigned int, len,
 					cmd_len - usb_rx_leftover_len);
@@ -2132,6 +2155,7 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 		if (remaining_bytes + usb_rx_leftover_len >
 						KVASER_USB_HYDRA_MAX_CMD_LEN) {
 			dev_err(&dev->intf->dev, "Format error\n");
+			card_data->usb_rx_leftover_len = 0;
 			spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
 			return;
 		}
@@ -2154,7 +2178,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 	while (pos < len) {
 		cmd = buf + pos;
 
-		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+		cmd_len = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos);
+		if (!cmd_len) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			return;
+		}
 
 		if (pos + cmd_len > len) {
 			/* We got first part of a command */
-- 
2.55.0

Re: [PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path

[sashiko-bot]

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

New issues:
- [High] The patch introduces a functional regression where valid fragmented USB streams are treated as format errors and dropped, permanently breaking stream reassembly.

Pre-existing issues:
- [High] The patch fails to validate that the length of a `CMD_EXTENDED` command is large enough for its payload, allowing out-of-bounds reads to persist in the handlers.
--

--- Patch [1]: [PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path ---
commit 1e011698d1b2d6b5a15d58005d752a57039bcf61
Author: Cen Zhang (Microsoft) <blbllhy@gmail.com>

can: kvaser_usb: validate command format before parsing in hydra receive path

This patch adds kvaser_usb_hydra_cmd_size_rx() to validate buffer bounds
before reading the command size. It attempts to fix a slab-out-of-bounds
read and an infinite loop when parsing malformed or zero-length CMD_EXTENDED
commands in the receive path.

Link: https://lore.kernel.org/all/20260819145658.29872-1-blbllhy@gmail.com

> 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..534bd26830125 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> @@ -536,6 +536,20 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
>  	return ret;
>  }
>  
> +/* Receive-path wrapper: validate buffer bounds before reading cmd_size. */
> +static size_t kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd,
> +					   size_t remaining)
> +{
> +	if (remaining < sizeof(struct kvaser_cmd_header))
> +		return 0;
> +
> +	if (cmd->header.cmd_no == CMD_EXTENDED &&
> +	    remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext))
> +		return 0;
> +
> +	return kvaser_usb_hydra_cmd_size(cmd);
> +}

[Severity: High]
This is a pre-existing issue, but does this patch completely fix the
out-of-bounds reads for extended commands?

If a malicious USB device sends a CMD_EXTENDED command placed at the end of a
bulk transfer (e.g., 6 bytes remaining at the end of a 3072-byte buffer) and
sets the len field to 6, kvaser_usb_hydra_cmd_size_rx() will pass the bounds
checks and return the device-controlled length of 6.

In kvaser_usb_hydra_read_bulk_callback(), the bounds check:

    if (pos + cmd_len > len)

will pass (3066 + 6 > 3072 is false). The command is then passed to
kvaser_usb_hydra_handle_cmd_ext(), which reads cmd->cmd_no_ext at offset 6
(out of bounds). 

If the command maps to CMD_RX_MESSAGE_FD, kvaser_usb_hydra_rx_msg_ext()
accesses cmd->rx_can.kcan_header at offset 20, causing a slab out-of-bounds
read that can be copied into a CAN frame payload and leaked to userspace.

Should there be an additional check to enforce that the returned cmd_len is
large enough to contain the extended struct payload?

> [ ... ]
> @@ -2154,7 +2178,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
>  	while (pos < len) {
>  		cmd = buf + pos;
>  
> -		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
> +		cmd_len = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos);
> +		if (!cmd_len) {
> +			dev_err(&dev->intf->dev, "Format error\n");
> +			return;
> +		}

[Severity: High]
Does this change break USB stream reassembly for valid fragmented commands?

Before this patch, if a standard command had only 2 bytes remaining at the end
of a URB, kvaser_usb_hydra_cmd_size() would determine it was not CMD_EXTENDED
and return sizeof(struct kvaser_cmd).

Those 2 bytes were then safely buffered in usb_rx_leftover and successfully
reassembled when the next URB arrived.

With this patch, kvaser_usb_hydra_cmd_size_rx() returns 0 if remaining < 4.
This causes the main loop in kvaser_usb_hydra_read_bulk_callback() to log
"Format error" and return immediately, discarding the fragment and permanently
desynchronizing the command stream.

>  
>  		if (pos + cmd_len > len) {
>  			/* We got first part of a command */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824214058.44948-1-blbllhy@gmail.com?part=1

Re: [PATCH net] can: kvaser_usb: validate command format before parsing in hydra receive path

[Cen Zhang (Microsoft)]

On Wed, Aug 19, 2026 at 03:10:26PM +0000, sashiko-bot@kernel.org wrote:
> This is a pre-existing issue, but do these error paths need to reset
> usb_rx_leftover_len to 0?

Yes. Otherwise the next transfer retries the same malformed leftover and
fails again. I will clear usb_rx_leftover_len before returning and send
v2.

> This isn't a bug introduced by this patch, but does the parser loop
> trust cmd_len without validating that it meets the minimum structural
> size expected by the command handler?

Yes. The generic parser validates framing, but not each command's minimum
structural length. That is a separate command-dispatch validation issue
and is outside the scope of this patch.

Thanks,
Cen

Re: [PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path

[Cen Zhang (Microsoft)]

Please disregard my previous reply; I accidentally sent the response to
the v1 review.

On Mon, Aug 24, 2026 at 09:53:43PM +0000, sashiko-bot@kernel.org wrote:
> Should there be an additional check to enforce that the returned
> cmd_len is large enough to contain the extended struct payload?

Yes, but this is a pre-existing command-specific structural validation
issue. Auditing each extended handler's minimum length is outside the scope
of this patch.

> Does this change break USB stream reassembly for valid fragmented
> commands?

Yes. An incomplete size field must be buffered rather than treated as a
format error. I will preserve incomplete fragments, complete an extended
size field before reading it, and send v3.

Thanks,
Cen