* [PATCH 1/2] can: esd_usb: validate received message length before use
2026-08-14 18:05 [PATCH 0/2] can: fix two missed siblings of the kvaser_usb_leaf receive-walk fix Yiran Qiu
@ 2026-08-14 18:05 ` Yiran Qiu
2026-08-14 18:05 ` [PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path Yiran Qiu
1 sibling, 0 replies; 4+ messages in thread
From: Yiran Qiu @ 2026-08-14 18:05 UTC (permalink / raw)
To: Frank Jungclaus, socketcan, Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, linux-kernel, Yiran Qiu, stable
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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] can: kvaser_usb_hydra: reject too-short commands in the receive path
2026-08-14 18:05 [PATCH 0/2] can: fix two missed siblings of the kvaser_usb_leaf receive-walk fix Yiran Qiu
2026-08-14 18:05 ` [PATCH 1/2] can: esd_usb: validate received message length before use Yiran Qiu
@ 2026-08-14 18:05 ` Yiran Qiu
2026-09-16 18:18 ` eritque-arcus
1 sibling, 1 reply; 4+ messages in thread
From: Yiran Qiu @ 2026-08-14 18:05 UTC (permalink / raw)
To: Frank Jungclaus, socketcan, Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, linux-kernel, Yiran Qiu, stable
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
^ permalink raw reply [flat|nested] 4+ messages in thread