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

[PATCH net v3] 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(), which checks whether the
size field is complete before reading it and rejects zero command lengths.
The asynchronous receive path preserves incomplete headers in the leftover
buffer and completes them from the following transfer.  Clear malformed
leftover state before returning so later transfers do not retry it.

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>
Link: https://lore.kernel.org/all/20260819145658.29872-1-blbllhy@gmail.com
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
v3:
 - Preserve fragmented command headers by distinguishing incomplete size
   fields from invalid zero lengths.
 - Complete a fragmented extended size field before reading it, using the
   actual valid leftover length.
 - Link: https://lore.kernel.org/all/20260824214058.44948-1-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 | 70 ++++++++++++++++---
 1 file changed, 62 insertions(+), 8 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..eddd207ee8da 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,24 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
 	return ret;
 }
 
+/* -EAGAIN means incomplete; -EINVAL rejects zero to ensure progress. */
+static int kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd,
+					size_t remaining, size_t *cmd_len)
+{
+	if (remaining < sizeof(cmd->header.cmd_no))
+		return -EAGAIN;
+
+	if (cmd->header.cmd_no == CMD_EXTENDED &&
+	    remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext))
+		return -EAGAIN;
+
+	*cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+	if (!*cmd_len)
+		return -EINVAL;
+
+	return 0;
+}
+
 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 +693,10 @@ 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) {
+			err = kvaser_usb_hydra_cmd_size_rx(tmp_cmd,
+							   actual_len - pos,
+							   &cmd_len);
+			if (err || pos + cmd_len > actual_len) {
 				dev_err_ratelimited(&dev->intf->dev,
 						    "Format error\n");
 				break;
@@ -2110,6 +2130,7 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 {
 	unsigned long irq_flags;
 	struct kvaser_cmd *cmd;
+	int err;
 	int pos = 0;
 	size_t cmd_len;
 	struct kvaser_usb_dev_card_data_hydra *card_data =
@@ -2120,24 +2141,53 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 	spin_lock_irqsave(usb_rx_leftover_lock, irq_flags);
 	usb_rx_leftover_len = card_data->usb_rx_leftover_len;
 	if (usb_rx_leftover_len) {
+		const size_t cmd_size_field_end =
+			offsetof(struct kvaser_cmd_ext, cmd_no_ext);
 		int remaining_bytes;
 
 		cmd = (struct kvaser_cmd *)card_data->usb_rx_leftover;
 
-		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+		if (cmd->header.cmd_no == CMD_EXTENDED &&
+		    usb_rx_leftover_len < cmd_size_field_end) {
+			remaining_bytes = min_t(int, len,
+						cmd_size_field_end -
+						usb_rx_leftover_len);
 
-		remaining_bytes = min_t(unsigned int, len,
+			memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len,
+			       buf, remaining_bytes);
+			usb_rx_leftover_len += remaining_bytes;
+			card_data->usb_rx_leftover_len = usb_rx_leftover_len;
+			pos += remaining_bytes;
+
+			if (usb_rx_leftover_len < cmd_size_field_end) {
+				spin_unlock_irqrestore(usb_rx_leftover_lock,
+						       irq_flags);
+				return;
+			}
+		}
+
+		err = kvaser_usb_hydra_cmd_size_rx(cmd, usb_rx_leftover_len,
+						   &cmd_len);
+		if (err || 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 - pos,
 					cmd_len - usb_rx_leftover_len);
 		/* Make sure we do not overflow usb_rx_leftover */
 		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;
 		}
 
-		memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len, buf,
-		       remaining_bytes);
+		memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len,
+		       buf + pos, remaining_bytes);
 		pos += remaining_bytes;
 
 		if (remaining_bytes + usb_rx_leftover_len == cmd_len) {
@@ -2154,9 +2204,13 @@ 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);
+		err = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos, &cmd_len);
+		if (err && err != -EAGAIN) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			return;
+		}
 
-		if (pos + cmd_len > len) {
+		if (err == -EAGAIN || pos + cmd_len > len) {
 			/* We got first part of a command */
 			int leftover_bytes;
 
-- 
2.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch prevents an OOB read of the `cmd->len` field but fails to enforce a minimum payload length for the extracted `cmd_len`, allowing maliciously truncated commands to bypass buffer bounds checks and cause slab-out-of-bounds reads in downstream handlers.
--

commit 25501dc93a8df5ec40c88edcab68f9d2a1cded89
Author: Cen Zhang (Microsoft) <blbllhy@gmail.com>

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

The receive-path command parsers call kvaser_usb_hydra_cmd_size() without
verifying that enough buffer remains. This patch adds
kvaser_usb_hydra_cmd_size_rx() to check whether the size field is complete
before reading it and rejects zero command lengths.

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..eddd207ee8daa 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,24 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
>  	return ret;
>  }
>  
> +/* -EAGAIN means incomplete; -EINVAL rejects zero to ensure progress. */
> +static int kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd,
> +					size_t remaining, size_t *cmd_len)
> +{
> +	if (remaining < sizeof(cmd->header.cmd_no))
> +		return -EAGAIN;
> +
> +	if (cmd->header.cmd_no == CMD_EXTENDED &&
> +	    remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext))
> +		return -EAGAIN;
> +
> +	*cmd_len = kvaser_usb_hydra_cmd_size(cmd);
> +	if (!*cmd_len)
> +		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does kvaser_usb_hydra_cmd_size_rx() need to
enforce a minimum payload length for the extracted cmd_len based on the
command type?

If a malicious USB device sends a CMD_EXTENDED packet with an artificially
small length (for example, 8), this function will extract that small length
without validating it against the actual structure size.

[ ... ]

> @@ -2154,9 +2204,13 @@ 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);
> +		err = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos, &cmd_len);
> +		if (err && err != -EAGAIN) {
> +			dev_err(&dev->intf->dev, "Format error\n");
> +			return;
> +		}
>  
> -		if (pos + cmd_len > len) {
> +		if (err == -EAGAIN || pos + cmd_len > len) {

[Severity: High]
This is a pre-existing issue, but because the extracted cmd_len is
artificially small, the buffer bounds check in
kvaser_usb_hydra_read_bulk_callback():

    if (err == -EAGAIN || pos + cmd_len > len) {

can pass when the packet is placed at the end of the URB buffer.

Downstream handlers like kvaser_usb_hydra_rx_msg_ext() will then read fields
at fixed offsets far exceeding this length, for instance:

    kcan_header = le32_to_cpu(cmd->rx_can.kcan_header);

at offset 20, or:

    memcpy(cf->data, cmd->rx_can.kcan_payload, cf->len);

at offset 32. Can this cause a slab-out-of-bounds read past the end of the USB
URB buffer?

>  			/* We got first part of a command */
>  			int leftover_bytes;

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

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

[Cen Zhang (Microsoft)]

On Wed, Aug 26, 2026 at 01:49:55AM +0000, sashiko-bot@kernel.org wrote:
> This is a pre-existing issue, but does kvaser_usb_hydra_cmd_size_rx()
> need to enforce a minimum payload length for the extracted cmd_len based
> on the command type?
>
> Can this cause a slab-out-of-bounds read past the end of the USB URB
> buffer?

This pre-existing command-specific structural validation issue was
discussed in the v2 review [1] and should be handled independently of this
patch.

No other issues were reported for v3.

Thanks,
Cen

[1] https://lore.kernel.org/all/20260825025011.13898-1-blbllhy@gmail.com/