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

[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