[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

[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

[eritque-arcus]

Hi Marc, Vincent,

Please drop patch 2/2 (kvaser_usb_hydra) from this series. Cen Zhang has 
since posted a standalone fix for the same hydra receive path that is 
more complete than mine.

<https://lore.kernel.org/linux-can/20260819145658.29872-1-blbllhy@gmail.com/>

Patch 1/2 (esd_usb) is independent so please consider it on its own.


One note on the automated review of patch 1/2: the issues it raised are 
all in esd_usb's probe and tx-done paths. I've kept this fix narrow 
rather than widen it into those, and will look at them separately.


Thanks,
Yiran

On 8/14/26 2:05 PM, Yiran Qiu wrote:
> 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;
>