[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

[PATCH net] 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.

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>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
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 | 34 ++++++++++++++++---
 1 file changed, 30 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..a964edde5783 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,14 @@ 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) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
+			return;
+		}
 
 		remaining_bytes = min_t(unsigned int, len,
 					cmd_len - usb_rx_leftover_len);
@@ -2154,7 +2176,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.52.0

Re: [PATCH net] 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:

Pre-existing issues:
- [High] Missing structural minimum length validation for CMD_EXTENDED payloads allows slab-out-of-bounds reads and potential kernel memory leaks.
- [High] Leftover buffer parsing errors fail to reset the buffer state, causing a permanent denial of service on the CAN interface.
--

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

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

This patch addresses slab-out-of-bounds reads and infinite loops in the
kvaser_usb hydra receive path. It introduces a receive-path wrapper,
kvaser_usb_hydra_cmd_size_rx(), to validate buffer bounds before reading
the command size.

> 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..a964edde5783c 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c

[ ... ]

> @@ -2124,7 +2139,14 @@ 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) {
> +			dev_err(&dev->intf->dev, "Format error\n");
> +			spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
> +			return;
> +		}

[Severity: High]
This is a pre-existing issue, but do these error paths need to reset
usb_rx_leftover_len to 0?

When an error is detected here (or in the subsequent pre-existing overflow 
check in kvaser_usb_hydra_read_bulk_callback), the code logs a format error,
drops the lock, and returns, but it fails to reset
card_data->usb_rx_leftover_len to 0.

On the next USB IN transfer, the non-zero leftover length forces execution back
into the leftover parsing block. The logic evaluates the exact same corrupted
command buffer, fails the exact same error check, and returns again.

Can this permanently wedge the parser and cause a denial of service on the CAN
interface?

[ ... ]

> @@ -2154,7 +2176,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 */

[Severity: High]
This isn't a bug introduced by this patch, but does the parser loop trust 
cmd_len (derived from the attacker-controlled cmd->len field) without
validating that it meets the minimum structural size expected by the command
handler?

For example, if a device claims a length of 8, the parser only ensures 8 bytes
are present. The command is then dispatched to handlers like 
kvaser_usb_hydra_rx_msg_ext(), which unconditionally read fields at fixed
offsets far beyond the claimed length:

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:kvaser_usb_hydra_rx_msg_ext()
    ...
    kcan_header = le32_to_cpu(cmd->rx_can.kcan_header);
    dlc = (kcan_header & KVASER_USB_KCAN_DATA_DLC_MASK) >>
        KVASER_USB_KCAN_DATA_DLC_SHIFT;
    ...
    memcpy(cf->data, cmd->rx_can.kcan_payload, cf->len);

Could this result in a slab-out-of-bounds read and potential kernel memory leak?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819145658.29872-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