can: kvaser_usb: validate command format before parsing in hydra receive path
PATCH v2 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/0x1240Additionally, 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. Clear invalid leftover state before returning so subsequent transfers do not retry the same malformed command.
---
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/[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 | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +32 -4
@@ -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,15 @@ 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 || 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, cmd_len - usb_rx_leftover_len);
@@ -2132,6 +2155,7 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,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 ; }
@@ -2154,7 +2178,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.55.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch introduces a functional regression where valid fragmented USB streams are treated as format errors and dropped, permanently breaking stream reassembly.
Pre-existing issues:
- [High] The patch fails to validate that the length of a CMD_EXTENDED command is large enough for its payload, allowing out-of-bounds reads to persist in the handlers. --
--- Patch [1]: [PATCH net v2] can: kvaser_usb: validate command format before parsing in hydra receive path --- commit 1e011698d1b2d6b5a15d58005d752a57039bcf61
Author: Cen Zhang (Microsoft) <[email protected]>
can: kvaser_usb: validate command format before parsing in hydra receive path
This patch adds kvaser_usb_hydra_cmd_size_rx() to validate buffer bounds before reading the command size. It attempts to fix a slab-out-of-bounds read and an infinite loop when parsing malformed or zero-length CMD_EXTENDED commands in the receive path.
Quoted 11 lines
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +3
-0@@ -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)
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd) · line 551
Quoted 10 lines
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +10
-0+{ +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); +}
[Severity: High]
This is a pre-existing issue, but does this patch completely fix the out-of-bounds reads for extended commands?
If a malicious USB device sends a CMD_EXTENDED command placed at the end of a bulk transfer (e.g., 6 bytes remaining at the end of a 3072-byte buffer) and sets the len field to 6, kvaser_usb_hydra_cmd_size_rx() will pass the bounds checks and return the device-controlled length of 6.
In kvaser_usb_hydra_read_bulk_callback(), the bounds check:
if (pos + cmd_len > len)
will pass (3066 + 6 > 3072 is false). The command is then passed to kvaser_usb_hydra_handle_cmd_ext(), which reads cmd->cmd_no_ext at offset 6 (out of bounds).
If the command maps to CMD_RX_MESSAGE_FD, kvaser_usb_hydra_rx_msg_ext() accesses cmd->rx_can.kcan_header at offset 20, causing a slab out-of-bounds read that can be copied into a CAN frame payload and leaked to userspace.
Should there be an additional check to enforce that the returned cmd_len is large enough to contain the extended struct payload?
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, · line 2185
Quoted 11 lines
[ ... ]
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +5
-1@@ -2154,7 +2178,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 ; + }
[Severity: High]
Does this change break USB stream reassembly for valid fragmented commands?
Before this patch, if a standard command had only 2 bytes remaining at the end of a URB, kvaser_usb_hydra_cmd_size() would determine it was not CMD_EXTENDED and return sizeof(struct kvaser_cmd).
Those 2 bytes were then safely buffered in usb_rx_leftover and successfully reassembled when the next URB arrived.
With this patch, kvaser_usb_hydra_cmd_size_rx() returns 0 if remaining < 4. This causes the main loop in kvaser_usb_hydra_read_bulk_callback() to log "Format error" and return immediately, discarding the fragment and permanently desynchronizing the command stream.
Quoted 3 lines
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +0
-0if (pos + cmd_len > len) {/* We got first part of a command */
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
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_lento 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_lenwithout 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
Please disregard my previous reply; I accidentally sent the response to the v1 review.
Quoted 2 lines · On Mon, Aug 24, 2026 at 09:53:43PM +0000, [email protected] wrote:
Should there be an additional check to enforce that the returned
cmd_lenis large enough to contain the extended struct payload?
Yes, but this is a pre-existing command-specific structural validation issue. Auditing each extended handler's minimum length is outside the scope of this patch.
Quoted 2 lines
Does this change break USB stream reassembly for valid fragmented commands?
Yes. An incomplete size field must be buffered rather than treated as a format error. I will preserve incomplete fragments, complete an extended size field before reading it, and send v3.
Thanks,
Cen