[PATCH 0/2] can: fix two missed siblings of the kvaser_usb_leaf receive-walk fix

[PATCH 0/2] can: fix two missed siblings of the kvaser_usb_leaf receive-walk fix

[Yiran Qiu]

Commit 0293dd153f9d ("can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd():
validate received command extents") fixed an unbounded variable-length
command walk in the kvaser_usb *leaf* receive paths. Two sibling USB-CAN
drivers have the same unbounded receive-buffer walk and were not touched by
that change:

  1. esd_usb: esd_usb_read_bulk_callback()'s only length check runs after the
     message has been dispatched and @pos advanced, so a short
     ESD_USB_CMD_CAN_RX header near the end of the buffer leads to an
     out-of-bounds read that is copied into a received CAN(-FD) skb
     (kernel-heap infoleak), and a zero-length message spins the URB
     completion softirq forever.

  2. kvaser_usb_hydra: kvaser_usb_hydra_read_bulk_callback() takes an
     extended command's length from the device with no lower bound, so a
     zero-length CMD_EXTENDED spins the URB completion softirq forever.

Both are reachable by a malicious or emulated USB CAN peripheral with no user
privileges (the driver auto-binds on probe), and both were reproduced with
USB_RAW_GADGET + dummy_hcd on a KASAN build; the per-patch changelogs carry
the splats. Only patch 1 (esd_usb) is memory-unsafe; patch 2 (hydra) is a
denial of service (soft lockup) only.

These were found by auditing the neighbourhood of 0293dd153f9d for the same
receive-walk shape and then reproducing each with a raw-gadget device. I can
send the gadget reproducers off-list on request.

Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
Yiran Qiu (2):
      can: esd_usb: validate received message length before use
      can: kvaser_usb_hydra: reject too-short commands in the receive path

 drivers/net/can/usb/esd_usb.c                     | 69 ++++++++++++++++++-----
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c |  9 +++
 2 files changed, 64 insertions(+), 14 deletions(-)
---
base-commit: 0d839570765118029aa8bf4a95444c6a11aacf85
change-id: 20260815-can-esd-hydra-fixes-86ac3eaef960

Best regards,
--  
Yiran Qiu <eritque-arcus@ikuyo.dev>

[PATCH 1/2] can: esd_usb: validate received message length before use

[Yiran Qiu]

esd_usb_read_bulk_callback() walks a sequence of variable-length
messages out of the RX URB buffer. The only length check,
"pos > urb->actual_length", runs *after* the current message has been
dispatched and after @pos has been advanced, so it can neither protect
the message being processed nor stop the loop:

 - The per-message handlers dereference fixed offsets of the message.
   esd_usb_rx_can_msg() reads the 32-bit CAN id at offset 8 and copies
   up to CANFD_MAX_DLEN payload bytes from offset 12; the error-event
   path reads four status bytes at offset 12; esd_usb_tx_done_msg()
   reads the tx handle at offset 4. A malicious or malfunctioning
   device can place a short ESD_USB_CMD_CAN_RX header near the end of
   actual_length so that these reads fall past the buffer, leaking
   adjacent kernel heap into a received CAN(-FD) skb.

 - hdr.len is the message length in 32-bit words. A message with
   hdr.len == 0 never advances @pos, spinning this URB-completion
   softirq forever.

Validate the header and the declared message length before dispatch:
reject a message whose header is not fully present, whose length is
zero, or which extends past the received data, and advance @pos by the
validated length. Pass the validated length to the message handlers so
they can confirm that the fields they read (and the payload they copy)
were actually received.

This is the esd_usb counterpart of the kvaser_usb_leaf fix,
commit 0293dd153f9d ("can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd():
validate received command extents"). esd_usb has the same unbounded
receive-buffer walk and was not touched by that fix.

Reproduced with USB_RAW_GADGET + dummy_hcd on a KASAN build: a bulk-IN
frame whose first message has hdr.len == 255 (advancing @pos to 1020)
followed by an ESD_USB_CMD_CAN_RX header at offset 1020 yields

  BUG: KASAN: slab-out-of-bounds in esd_usb_read_bulk_callback+0x38f/0xd70
  Read of size 4 at addr ffff88800c0a9c04 by task init/1
   kasan_check_range
   esd_usb_read_bulk_callback+0x38f/0xd70
   __usb_hcd_giveback_urb
   dummy_timer
  The buggy address belongs to the object at ffff88800c0a9800
   which belongs to the cache kmalloc-1k of size 1024
  The buggy address is located 4 bytes to the right of
   allocated 1024-byte region [ffff88800c0a9800, ffff88800c0a9c00)

Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
Cc: stable@vger.kernel.org
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
 drivers/net/can/usb/esd_usb.c | 69 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
index f41d4a0d140f7..13356e68f3f60 100644
--- a/drivers/net/can/usb/esd_usb.c
+++ b/drivers/net/can/usb/esd_usb.c
@@ -298,7 +298,7 @@ struct esd_usb_net_priv {
 };
 
 static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
-			     union esd_usb_msg *msg)
+			     union esd_usb_msg *msg, unsigned int msg_len)
 {
 	struct net_device_stats *stats = &priv->netdev->stats;
 	struct can_frame *cf;
@@ -306,8 +306,14 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
 	u32 id = le32_to_cpu(msg->rx.id) & ESD_USB_IDMASK;
 
 	if (id == ESD_USB_EV_CAN_ERROR_EXT) {
-		u8 state = msg->rx.ev_can_err_ext.status;
-		u8 ecc = msg->rx.ev_can_err_ext.ecc;
+		u8 state;
+		u8 ecc;
+
+		if (msg_len < offsetofend(struct esd_usb_rx_msg, ev_can_err_ext))
+			return;
+
+		state = msg->rx.ev_can_err_ext.status;
+		ecc = msg->rx.ev_can_err_ext.ecc;
 
 		priv->bec.rxerr = msg->rx.ev_can_err_ext.rec;
 		priv->bec.txerr = msg->rx.ev_can_err_ext.tec;
@@ -395,7 +401,7 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
 }
 
 static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
-			       union esd_usb_msg *msg)
+			       union esd_usb_msg *msg, unsigned int msg_len)
 {
 	struct net_device_stats *stats = &priv->netdev->stats;
 	struct can_frame *cf;
@@ -407,10 +413,19 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
 	if (!netif_device_present(priv->netdev))
 		return;
 
+	/* The device controls the message length; make sure the fixed rx
+	 * header (up to and including the CAN id) was actually received
+	 * before it is dereferenced.
+	 */
+	if (msg_len < offsetofend(struct esd_usb_rx_msg, id)) {
+		stats->rx_length_errors++;
+		return;
+	}
+
 	id = le32_to_cpu(msg->rx.id);
 
 	if (id & ESD_USB_EVENT) {
-		esd_usb_rx_event(priv, msg);
+		esd_usb_rx_event(priv, msg, msg_len);
 	} else {
 		if (msg->rx.dlc & ESD_USB_FD) {
 			skb = alloc_canfd_skb(priv->netdev, &cfd);
@@ -446,6 +461,15 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
 		if (id & ESD_USB_EXTID)
 			cfd->can_id |= CAN_EFF_FLAG;
 
+		/* Reject a frame that claims more payload than was actually
+		 * received, to avoid copying past the URB buffer.
+		 */
+		if (len > msg_len - offsetofend(struct esd_usb_rx_msg, id)) {
+			stats->rx_length_errors++;
+			dev_kfree_skb_any(skb);
+			return;
+		}
+
 		memcpy(cfd->data, msg->rx.data_fd, len);
 		stats->rx_bytes += len;
 		stats->rx_packets++;
@@ -455,7 +479,7 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
 }
 
 static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
-				union esd_usb_msg *msg)
+				union esd_usb_msg *msg, unsigned int msg_len)
 {
 	struct net_device_stats *stats = &priv->netdev->stats;
 	struct net_device *netdev = priv->netdev;
@@ -464,6 +488,9 @@ static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
 	if (!netif_device_present(netdev))
 		return;
 
+	if (msg_len < offsetofend(struct esd_usb_tx_done_msg, hnd))
+		return;
+
 	context = &priv->tx_contexts[msg->txdone.hnd & (ESD_USB_MAX_TX_URBS - 1)];
 
 	if (!msg->txdone.status) {
@@ -507,8 +534,27 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 
 	while (pos < urb->actual_length) {
 		union esd_usb_msg *msg;
+		unsigned int msg_len;
+
+		/* The header must be fully present before hdr.len / hdr.cmd
+		 * (and the net index below) are read.
+		 */
+		if (pos + sizeof(struct esd_usb_header_msg) > urb->actual_length) {
+			dev_err(dev->udev->dev.parent, "format error\n");
+			break;
+		}
 
 		msg = (union esd_usb_msg *)(urb->transfer_buffer + pos);
+		msg_len = msg->hdr.len * sizeof(u32); /* convert to # of bytes */
+
+		/* A zero-length message would never advance @pos and would
+		 * spin this URB-completion softirq forever; a message must
+		 * also fit within the received data.
+		 */
+		if (msg->hdr.len == 0 || msg_len > urb->actual_length - pos) {
+			dev_err(dev->udev->dev.parent, "format error\n");
+			break;
+		}
 
 		switch (msg->hdr.cmd) {
 		case ESD_USB_CMD_CAN_RX:
@@ -517,7 +563,7 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 				break;
 			}
 
-			esd_usb_rx_can_msg(dev->nets[msg->rx.net], msg);
+			esd_usb_rx_can_msg(dev->nets[msg->rx.net], msg, msg_len);
 			break;
 
 		case ESD_USB_CMD_CAN_TX:
@@ -527,16 +573,11 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 			}
 
 			esd_usb_tx_done_msg(dev->nets[msg->txdone.net],
-					    msg);
+					    msg, msg_len);
 			break;
 		}
 
-		pos += msg->hdr.len * sizeof(u32); /* convert to # of bytes */
-
-		if (pos > urb->actual_length) {
-			dev_err(dev->udev->dev.parent, "format error\n");
-			break;
-		}
+		pos += msg_len;
 	}
 
 resubmit_urb:

-- 
2.55.0

Re: [PATCH 1/2] can: esd_usb: validate received message length before use

[sashiko-bot]

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

Pre-existing issues:
- [High] Unvalidated interface count from device causes heap buffer overflow during probe.
- [High] Dereferencing uninitialized network interfaces from USB messages causes kernel panic.
- [High] Spurious TX_DONE messages decrement inactive contexts causing active_tx_jobs underflow.
--

--- Patch [1]: [PATCH 1/2] can: esd_usb: validate received message length before use ---
commit 38efd7f38912ccfd78688896fcdf77da350528fd
Author: Yiran Qiu <eritque-arcus@ikuyo.dev>
Date:   Fri Aug 14 18:05:46 2026 +0000

    can: esd_usb: validate received message length before use
    
    esd_usb_read_bulk_callback() walks a sequence of variable-length
    messages out of the RX URB buffer. The only length check,
    "pos > urb->actual_length", runs *after* the current message has been
    dispatched and after @pos has been advanced, so it can neither protect
    the message being processed nor stop the loop:
    
    Validate the header and the declared message length before dispatch:
    reject a message whose header is not fully present, whose length is
    zero, or which extends past the received data, and advance @pos by the
    validated length. Pass the validated length to the message handlers so
    they can confirm that the fields they read (and the payload they copy)
    were actually received.

> diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
> index f41d4a0d140f7..13356e68f3f60 100644
> --- a/drivers/net/can/usb/esd_usb.c
> +++ b/drivers/net/can/usb/esd_usb.c

[ ... ]

> @@ -455,7 +479,7 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
>  }
>  
>  static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
> -				union esd_usb_msg *msg)
> +				union esd_usb_msg *msg, unsigned int msg_len)
>  {
>  	struct net_device_stats *stats = &priv->netdev->stats;
>  	struct net_device *netdev = priv->netdev;
> @@ -464,6 +488,9 @@ static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
>  	if (!netif_device_present(netdev))
>  		return;
>  
> +	if (msg_len < offsetofend(struct esd_usb_tx_done_msg, hnd))
> +		return;
> +
>  	context = &priv->tx_contexts[msg->txdone.hnd & (ESD_USB_MAX_TX_URBS - 1)];
>  
>  	if (!msg->txdone.status) {

[Severity: High]
This is a pre-existing issue, but can spurious TX_DONE messages decrement
inactive contexts and cause the active_tx_jobs counter to underflow?

In esd_usb_tx_done_msg(), the context identified by msg->txdone.hnd is accessed
without checking if it is actually active (e.g. echo_index !=
ESD_USB_MAX_TX_URBS). If a malicious device sends spurious TX_DONE messages,
this will unconditionally execute atomic_dec(&priv->active_tx_jobs). 

When the queue eventually fills up with real packets, esd_usb_start_xmit()
won't stop the queue because active_tx_jobs is negative, returning
NETDEV_TX_BUSY. Returning NETDEV_TX_BUSY without stopping the queue causes the
network stack to infinitely retry, softlocking the CPU.

[ ... ]

> @@ -507,8 +534,27 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
>  
>  	while (pos < urb->actual_length) {
>  		union esd_usb_msg *msg;
> +		unsigned int msg_len;
> +
> +		/* The header must be fully present before hdr.len / hdr.cmd
> +		 * (and the net index below) are read.
> +		 */
> +		if (pos + sizeof(struct esd_usb_header_msg) > urb->actual_length) {
> +			dev_err(dev->udev->dev.parent, "format error\n");
> +			break;
> +		}
>  
>  		msg = (union esd_usb_msg *)(urb->transfer_buffer + pos);
> +		msg_len = msg->hdr.len * sizeof(u32); /* convert to # of bytes */
> +
> +		/* A zero-length message would never advance @pos and would
> +		 * spin this URB-completion softirq forever; a message must
> +		 * also fit within the received data.
> +		 */
> +		if (msg->hdr.len == 0 || msg_len > urb->actual_length - pos) {
> +			dev_err(dev->udev->dev.parent, "format error\n");
> +			break;
> +		}
>  
>  		switch (msg->hdr.cmd) {
>  		case ESD_USB_CMD_CAN_RX:
> @@ -517,7 +563,7 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
>  				break;
>  			}
>  
> -			esd_usb_rx_can_msg(dev->nets[msg->rx.net], msg);
> +			esd_usb_rx_can_msg(dev->nets[msg->rx.net], msg, msg_len);

[Severity: High]
This isn't a bug introduced by this patch, but does this loop properly handle
uninitialized network interfaces?

If esd_usb_probe_one_net() fails for one of the interfaces during
esd_usb_probe(), it leaves dev->nets[index] as NULL. A malicious USB device can
then send ESD_USB_CMD_CAN_RX messages with the index of the failed interface,
which is passed unconditionally to esd_usb_rx_can_msg() resulting in a NULL
pointer dereference.

[Severity: High]
This is also a pre-existing issue, but does msg->rx.net >= dev->net_count
properly protect against out-of-bounds array access for dev->nets[]?

A malicious USB device can return an arbitrarily large number of interfaces in
the version reply message. In esd_usb_probe(), this is directly assigned to
dev->net_count without validating it against ESD_USB_MAX_NETS (2), which can
corrupt adjacent heap memory when writing to dev->nets[index]. Furthermore,
since dev->net_count can be larger than ESD_USB_MAX_NETS, the check
msg->rx.net >= dev->net_count above does not prevent dev->nets[msg->rx.net]
from going out of bounds here.

>  			break;
>  
>  		case ESD_USB_CMD_CAN_TX:
> @@ -527,16 +573,11 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
>  			}
>  
>  			esd_usb_tx_done_msg(dev->nets[msg->txdone.net],
> -					    msg);
> +					    msg, msg_len);
>  			break;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-can-esd-hydra-fixes-v1-0-de644cbeaec2@ikuyo.dev?part=1

[PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path

[Yiran Qiu]

kvaser_usb_hydra_read_bulk_callback() walks commands out of the RX URB
buffer, using kvaser_usb_hydra_cmd_size() to determine each command's
length. For an extended command (CMD_EXTENDED) that size is taken
directly from the device-supplied 16-bit length field with no lower
bound. A CMD_EXTENDED command whose length is zero makes cmd_size 0, so
"pos += cmd_len" never advances and this URB-completion softirq spins
forever.

Reject a command whose reported size is smaller than the command header
before it is dispatched, mirroring the minimum-length check added in
commit 0293dd153f9d ("can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd():
validate received command extents"); that fix did not touch hydra's
asynchronous read_bulk_callback().

Reproduced with USB_RAW_GADGET + dummy_hcd on a KASAN build: after the
normal probe/START_CHIP handshake, a 6-byte CMD_EXTENDED frame with the
length field set to 0 makes the callback loop print

  kvaser_usb 1-1:1.0: Unhandled extended command (255)

without bound (306000 times in ~75 s), until

  rcu: INFO: rcu_sched detected stalls on CPUs/tasks:

and the machine had to be killed externally.

Fixes: aec5fb2268b5 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Cc: stable@vger.kernel.org
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 9 +++++++++
 1 file changed, 9 insertions(+)

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..d44f9875fbe2f 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -2156,6 +2156,15 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 
 		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
 
+		/* An extended command carries a device-supplied length; a
+		 * command shorter than the command header would never advance
+		 * @pos and would spin this URB-completion softirq forever.
+		 */
+		if (cmd_len < sizeof(struct kvaser_cmd_header)) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			break;
+		}
+
 		if (pos + cmd_len > len) {
 			/* We got first part of a command */
 			int leftover_bytes;

-- 
2.55.0

Re: [PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Processing an unexpected or duplicate TX acknowledge command causes an out-of-bounds array access, leading to memory corruption.
- [High] kvaser_usb_hydra_cmd_size() reads the command length from the URB buffer without verifying sufficient data is available, and the patch's newly added minimum length check is insufficient to prevent out-of-bounds accesses during command dispatch.
- [High] The patch fails to validate command lengths in the leftover buffer path, leading to an integer underflow, and an error path fails to reset the leftover length, permanently wedging the receive path.
- [High] The dev_get_berr_counter implementation synchronously transmits a USB command using GFP_KERNEL allocations while running inside an RCU read-side critical section.
- [High] The driver blindly copies CAN frame payload data based on the device-supplied DLC without verifying that the command length is large enough, leading to a heap information leak.
- [High] The patch fixes an infinite loop in the URB completion callback but leaves the exact same vulnerability in the synchronous command wait function.
--

--- Patch [2]: [PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path ---
commit c7aa22e9c2e49b7c5c3858b9afe989871f77d49c
Author: Yiran Qiu <eritque-arcus@ikuyo.dev>

can: kvaser_usb_hydra: reject too-short commands in the receive path

This commit fixes an infinite loop in the URB-completion softirq caused by
extended commands with a length of zero. It adds a minimum-length check before
dispatching commands.

Cc: stable@vger.kernel.org

> 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..d44f9875fbe2f 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> @@ -2156,6 +2156,15 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
>  
>  		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
>  

[Severity: High]
This isn't a bug introduced by this patch, but does the call to
kvaser_usb_hydra_cmd_size() read the command length from the URB buffer
without verifying sufficient data is available?

If a malicious device sends an extended command split across URBs, or one with
a length of 4 or 5 bytes, kvaser_usb_hydra_cmd_size() reads the 16-bit length
field at offset 4:

kvaser_usb_hydra_cmd_size()
    ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);

This occurs without checking if the buffer has enough data, leading to an
out-of-bounds read.

> +		/* An extended command carries a device-supplied length; a
> +		 * command shorter than the command header would never advance
> +		 * @pos and would spin this URB-completion softirq forever.
> +		 */
> +		if (cmd_len < sizeof(struct kvaser_cmd_header)) {
> +			dev_err(&dev->intf->dev, "Format error\n");
> +			break;
> +		}
> +

[Severity: High]
This is a pre-existing issue, but is the validation check added here
sufficient to prevent out-of-bounds accesses during command dispatch?

Since a command length of 4 or 5 passes this check, it allows
kvaser_usb_hydra_handle_cmd_ext() to be called. That function then reads
cmd_no_ext at offset 6:

kvaser_usb_hydra_handle_cmd_ext()
    switch (cmd->cmd_no_ext) {

Could this lead to an out-of-bounds array access on the URB buffer and
corrupted command processing?

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

[Severity: High]
This isn't a bug introduced by this patch, but earlier in
kvaser_usb_hydra_read_bulk_callback(), does the leftover buffer path fail to
validate command lengths?

If the leftover buffer evaluates cmd_len to 0, cmd_len - usb_rx_leftover_len
underflows to a massive unsigned integer. Once usb_rx_leftover_len
accumulates past 128 bytes, the driver prints an error and returns:

kvaser_usb_hydra_read_bulk_callback()
    if (remaining_bytes + usb_rx_leftover_len >
                                    KVASER_USB_HYDRA_MAX_CMD_LEN) {
            dev_err(&dev->intf->dev, "Format error\n");
            spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
            return;
    }

Crucially, it fails to reset usb_rx_leftover_len to 0. Would every subsequent
URB immediately hit this exact overflow condition and be dropped, permanently
wedging the device receive path?

[Severity: High]
This is a pre-existing issue, but does kvaser_usb_hydra_wait_cmd() contain the
exact same infinite loop vulnerability that this patch fixes?

kvaser_usb_hydra_wait_cmd() iterates over incoming USB data using a similar
loop. A malicious device could send an extended command with length 0, causing
cmd_len to evaluate to 0:

kvaser_usb_hydra_wait_cmd()
    pos += cmd_len;

Since the command is not the expected response, the loop adds 0 to pos and
spins forever without advancing. Will this lock up the kernel during device
probe or configuration?

[Severity: High]
This isn't a bug introduced by this patch, but does the
dev_get_berr_counter implementation synchronously transmit a USB command
using GFP_KERNEL allocations while running inside an RCU read-side critical
section?

The callback is invoked by can_fill_info() when responding to netlink
RTM_GETLINK dumps, which execute under rcu_read_lock(). Inside this RCU
critical section, kvaser_usb_hydra_get_berr_counter() dynamically allocates
memory with GFP_KERNEL and calls wait_for_completion_timeout() to submit a
synchronous URB:

kvaser_usb_hydra_get_berr_counter()
    err = kvaser_usb_hydra_send_simple_cmd(priv->dev,
                                           CMD_GET_CHIP_STATE_REQ,
                                           priv->channel);

Could this violate core kernel locking rules, leading to sleeping in atomic
context, lockdep splats, and potential RCU stalls?

[Severity: High]
This is a pre-existing issue, but does the driver blindly copy CAN frame
payload data based on the device-supplied DLC without verifying the command
length?

In kvaser_usb_hydra_rx_msg_ext(), the driver extracts the DLC and converts it
to a payload length. It then blindly copies cf->len bytes from the URB buffer:

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

If a malicious device sends an extended command with a small USB transfer
length but a large DLC, will the driver read out-of-bounds past the end of
the USB transfer buffer, and leak kernel heap memory to userspace?

[Severity: Critical]
This isn't a bug introduced by this patch, but does processing an
unexpected or duplicate TX acknowledge command cause an out-of-bounds array
access?

When processing a duplicate TX ACK, context->echo_index is reset to
dev->max_tx_urbs, which matches the size of the echo_skb array. The driver
directly accesses priv->can.echo_skb[context->echo_index] without bounds
checking:

kvaser_usb_hydra_tx_acknowledge()
    skb = priv->can.echo_skb[context->echo_index];
    if (skb)
            skb_hwtstamps(skb)->hwtstamp =
                               kvaser_usb_hydra_ktime_from_cmd(dev->cfg, cmd);

This accesses exactly one element past the end of the dynamically allocated
echo_skb array. If the out-of-bounds memory contains a non-zero value, it is
treated as a valid sk_buff pointer, and an 8-byte timestamp is written to it.
Could this lead to wild pointer writes and exploitable kernel memory
corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-can-esd-hydra-fixes-v1-0-de644cbeaec2@ikuyo.dev?part=2