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

PATCH Superseded Cen Zhang (Microsoft) 2 participants 3 messages Aug 19 – Aug 24 CAN network drivers

Share & tools

Cen Zhang (Microsoft)

author

2026-08-19 14:57 UTC #0 lore raw

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

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 [email protected]

Reported-by Xiang Mei (Microsoft) <[email protected]>

Reported-by Cen Zhang (Microsoft) <[email protected]>

Signed-off-by Cen Zhang (Microsoft) <[email protected]>

---
The zero-length command loop is also addressed by:
https://lore.kernel.org/linux-can/[email protected]/

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(-)

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +30 -4

index efbb7bed34c9..a964edde5783 100644
@@ -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 */

Signature · 1 line

2.52.0

[email protected]

AI review

2026-08-19 15:10 UTC #1 lore raw

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) <[email protected]>

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.

Quoted 4 lines

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +0 -0
index efbb7bed34c9d..a964edde5783c 100644

[ ... ]

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, · line 2149

Quoted 13 lines

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +8 -1

@@ -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?

[ ... ]

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, · line 2186

Quoted 13 lines

drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +5 -1

@@ -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?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Cen Zhang (Microsoft)

author

2026-08-24 21:35 UTC #2 lore raw

Quoted 2 lines · On Wed, Aug 19, 2026 at 03:10:26PM +0000, [email protected] 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.

Quoted 3 lines

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