From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org, linux-can@vger.kernel.org,
kernel@pengutronix.de,
"Cen Zhang (Microsoft Security FORGE Labs)"
<cenzhang@linux.microsoft.com>,
AutonomousCodeSecurity@microsoft.com,
"Xiang Mei (Microsoft)" <xmei5@asu.edu>,
stable@vger.kernel.org, Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH net-next 32/37] can: kvaser_usb: validate command format before parsing in hydra receive path
Date: Fri, 9 Oct 2026 15:28:04 +0200 [thread overview]
Message-ID: <20261009134323.64064-33-mkl@pengutronix.de> (raw)
In-Reply-To: <20261009134323.64064-1-mkl@pengutronix.de>
From: "Cen Zhang (Microsoft Security FORGE Labs)" <cenzhang@linux.microsoft.com>
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. 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.
A positive but undersized extended length can also pass the buffer extent
check and reach a handler. For example, an 8-byte CMD_RX_MESSAGE_FD at
the end of an RX URB causes kvaser_usb_hydra_rx_msg_ext() to read fixed
fields and payload past the buffer.
Add receive-side length validation which preserves incomplete headers for
reassembly, rejects extended lengths outside 8..128 bytes, and checks each
known extended command against the minimum length its handler consumes.
For CMD_RX_MESSAGE_FD, derive the required length from its flags and DLC
so valid variable-length commands remain accepted. Apply the checks to
both receive paths and clear malformed leftover state before returning.
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>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Closes: https://lore.kernel.org/all/20260819145658.29872-1-blbllhy@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Link: https://patch.msgid.link/20260910135000.34796-1-cenzhang@linux.microsoft.com
[mkl: reduce scope of err in kvaser_usb_hydra_read_bulk_callback()]
[mkl: increase readability, reformat to make use of ~100 columns]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
.../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 135 +++++++++++++++++-
1 file changed, 128 insertions(+), 7 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 ad168cad1d78..68bbccb94bb1 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -533,6 +533,82 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
return ret;
}
+/* -EAGAIN means incomplete; -EINVAL rejects an invalid command length. */
+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->header.cmd_no != CMD_EXTENDED)
+ return 0;
+
+ if (*cmd_len < offsetof(struct kvaser_cmd_ext, rx_can) ||
+ *cmd_len > KVASER_USB_HYDRA_MAX_CMD_LEN)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int kvaser_usb_hydra_verify_cmd_size(const struct kvaser_cmd *cmd,
+ size_t cmd_len)
+{
+ const struct kvaser_cmd_ext *cmd_ext;
+ size_t min_len;
+
+ if (cmd->header.cmd_no != CMD_EXTENDED)
+ return 0;
+
+ cmd_ext = (const struct kvaser_cmd_ext *)cmd;
+
+ /* Keep this switch in sync with kvaser_usb_hydra_handle_cmd_ext(). */
+ switch (cmd_ext->cmd_no_ext) {
+ case CMD_TX_ACKNOWLEDGE_FD:
+ min_len = offsetof(struct kvaser_cmd_ext, tx_ack.timestamp) +
+ sizeof(cmd_ext->tx_ack.timestamp);
+ break;
+
+ case CMD_RX_MESSAGE_FD: {
+ u32 flags;
+
+ min_len = offsetof(struct kvaser_cmd_ext, rx_can.kcan_payload);
+ if (cmd_len < min_len)
+ return -EINVAL;
+
+ flags = le32_to_cpu(cmd_ext->rx_can.flags);
+ if (flags & KVASER_USB_HYDRA_CF_FLAG_ERROR_FRAME) {
+ min_len += sizeof(cmd_ext->rx_can.err_frame_data);
+ } else if (!(flags & KVASER_USB_HYDRA_CF_FLAG_REMOTE_FRAME)) {
+ u32 kcan_header;
+ u8 dlc;
+
+ kcan_header = le32_to_cpu(cmd_ext->rx_can.kcan_header);
+ dlc = (kcan_header & KVASER_USB_KCAN_DATA_DLC_MASK) >>
+ KVASER_USB_KCAN_DATA_DLC_SHIFT;
+
+ if (flags & KVASER_USB_HYDRA_CF_FLAG_FDF)
+ min_len += can_fd_dlc2len(dlc);
+ else
+ min_len += can_cc_dlc2len(dlc);
+ }
+ break;
+ }
+
+ default:
+ return 0;
+ }
+
+ if (cmd_len < min_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)
@@ -672,8 +748,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) {
+ err = kvaser_usb_hydra_cmd_size_rx(tmp_cmd, actual_len - pos,&cmd_len);
+ if (err || pos + cmd_len > actual_len ||
+ kvaser_usb_hydra_verify_cmd_size(tmp_cmd, cmd_len)) {
dev_err_ratelimited(&dev->intf->dev,
"Format error\n");
break;
@@ -2091,27 +2168,60 @@ 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;
+ int err;
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,
+ 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) {
+ if (kvaser_usb_hydra_verify_cmd_size(cmd, 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;
+ }
+
kvaser_usb_hydra_handle_cmd(dev, cmd);
usb_rx_leftover_len = 0;
} else {
@@ -2123,11 +2233,17 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
while (pos < len) {
+ int err;
+
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;
@@ -2145,6 +2261,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
break;
}
+ if (kvaser_usb_hydra_verify_cmd_size(cmd, cmd_len)) {
+ dev_err(&dev->intf->dev, "Format error\n");
+ return;
+ }
+
kvaser_usb_hydra_handle_cmd(dev, cmd);
pos += cmd_len;
}
--
2.53.0
next prev parent reply other threads:[~2026-10-09 13:43 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-09 13:27 [PATCH net-next 0/37] pull-request: can-next 2026-10-09 Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 01/37] can: dev: can_dropped_invalid_skb: drop CAN XL frames on non-CAN XL devices Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 02/37] can: raw: remove redundant NULL check before netdev_hold() Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 03/37] can: convert unreliable ARPHRD_CAN type checks to robust can_get_ml_priv() Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 04/37] can: proc: reset pkg_stats atomics individually Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 05/37] can: proc: remove pointers from CAN specific proc output Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 06/37] can: j1939: cancel pending address claim timers from j1939_ecu_unmap_all() Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 07/37] can: isotp: check the frame type, not just the length Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 08/37] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 09/37] can: rcar_canfd: Fix typos in macro names Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 10/37] can: skb: make echo skb freeing safe in any IRQ context Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 11/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 12/37] can: skb: make CAN skb allocation failure paths IRQ-safe Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 13/37] can: rcar_canfd: Do not set registers selecting the CAN mode Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 14/37] can: dev: can_put_echo_skb(): free skb on invalid echo index Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 15/37] can: rcar_canfd: Add support for Renesas RZ/G3S Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 16/37] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3L SoC Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 17/37] can: rcar_canfd: Derive max_channels from the device tree Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 18/37] dt-bindings: net: can: convert grcan to DT schema Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 19/37] can: rcar_canfd: Add support for Renesas RZ/G3L Marc Kleine-Budde
2026-10-10 14:02 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 20/37] dt-bindings: can: renesas,rcar-canfd: Restrict resets in top-level Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 21/37] can: grcan: update the binding file reference in the driver comment Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 22/37] can: remove Softing CANcard driver Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 23/37] can: Convert to DEFINE_SIMPLE_DEV_PM_OPS() Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 24/37] can: cc770: don't discard the IRQ lookup error in probe Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 25/37] can: cc770: fix the clock divider check on the platform bus Marc Kleine-Budde
2026-10-09 13:27 ` [PATCH net-next 26/37] can: ems_usb: use usb_kill_urb() to stop the intr URB Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:27 ` [PATCH net-next 27/37] can: esd: acc_start_xmit(): do not touch skb after can_put_echo_skb() Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:28 ` [PATCH net-next 28/37] can: flexcan: flexcan_setup_stop_mode_gpr: fix OF node reference leak Marc Kleine-Budde
2026-10-09 13:28 ` [PATCH net-next 29/37] can: f81604: f81604_close(): fix use-after-free on disconnect Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:28 ` [PATCH net-next 30/37] can: hi311x: drop hi3110_lock before free_irq() on open failure Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:28 ` [PATCH net-next 31/37] can: kvaser_usb: refactor endpoint lookup Marc Kleine-Budde
2026-10-09 13:28 ` Marc Kleine-Budde [this message]
2026-10-09 13:28 ` [PATCH net-next 33/37] can: kvaser_pciefd: fix use-after-free in bec poll timer Marc Kleine-Budde
2026-10-09 13:28 ` [PATCH net-next 34/37] can: mcp251xfd: mcp251xfd_probe(): reject devices without match data Marc Kleine-Budde
2026-10-09 13:28 ` [PATCH net-next 35/37] can: sun4i_can: sun4ican_probe(): fix clk leak Marc Kleine-Budde
2026-10-10 14:03 ` netdev-bot+sashiko
2026-10-09 13:28 ` [PATCH net-next 36/37] can: ucan: fix repeated word 'is' in comment Marc Kleine-Budde
2026-10-09 13:28 ` [PATCH net-next 37/37] can: xilinx_can: set CAN FD flags on received frames Marc Kleine-Budde
2026-10-09 13:57 ` [PATCH net-next 0/37] pull-request: can-next 2026-10-09 Marc Kleine-Budde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261009134323.64064-33-mkl@pengutronix.de \
--to=mkl@pengutronix.de \
--cc=AutonomousCodeSecurity@microsoft.com \
--cc=cenzhang@linux.microsoft.com \
--cc=davem@davemloft.net \
--cc=kernel@pengutronix.de \
--cc=kuba@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=xmei5@asu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox