* [PATCH v3 1/4] can: kvaser_usb: hydra: reject commands shorter than a standard command
2026-11-12 17:27 [PATCH v3 0/4] can: kvaser_usb: validate values received from the device Joas Antonio dos Santos via B4 Relay
@ 2026-11-12 17:27 ` Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 2/4] can: kvaser_usb: reject devices reporting zero max outstanding tx Joas Antonio dos Santos via B4 Relay
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Joas Antonio dos Santos via B4 Relay @ 2026-11-12 17:27 UTC (permalink / raw)
To: mkl, mailhol, joasantonio108; +Cc: linux-can, linux-kernel, stable
From: Joas Antonio dos Santos <joasantonio108@gmail.com>
kvaser_usb_hydra_cmd_size() returns the length field supplied by the
device for extended commands. Both command parsers advance through the
received buffer by that length:
kvaser_usb_hydra_wait_cmd() pos += cmd_len;
kvaser_usb_hydra_read_bulk_callback() pos += cmd_len;
Neither checks that the length is non-zero. A device that sends an
extended command (cmd_no == CMD_EXTENDED) with a length of zero keeps
pos unchanged, so the loop never terminates. In
kvaser_usb_hydra_read_bulk_callback() this happens in the URB completion
handler, which also calls kvaser_usb_hydra_handle_cmd() on the same
command in every iteration, and locks up the CPU. In
kvaser_usb_hydra_wait_cmd() the loop spins in process context without
ever reaching the timeout check.
A non-zero length that is too small is not valid either. The parsers
only check that cmd_len bytes are available, but the handlers of the
extended commands read the fixed part of the command without looking at
the length again: cmd_no_ext, the flags and timestamp of
struct kvaser_cmd_ext_tx_ack and the fields of
struct kvaser_cmd_ext_rx_can in front of the payload. That fixed part
ends at byte 32 in both cases, so a shorter extended command makes the
handlers read bytes that are not part of the command.
Standard commands are sizeof(struct kvaser_cmd), 32 bytes, and the
extended commands that the driver handles or sends are never shorter
than that. Treat a shorter length as a format error in both parsers.
A similar infinite loop in the leaf command parsers was fixed in
commit 0c73772cd2b8 ("can: kvaser_usb: leaf: Fix potential infinite loop
in command parsers").
Found by LLM-assisted source review.
Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
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 687911d764de..9adb4eb36e39 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -676,7 +676,8 @@ static int kvaser_usb_hydra_wait_cmd(const struct kvaser_usb *dev, u8 cmd_no,
tmp_cmd = buf + pos;
cmd_len = kvaser_usb_hydra_cmd_size(tmp_cmd);
- if (pos + cmd_len > actual_len) {
+ if (cmd_len < sizeof(struct kvaser_cmd) ||
+ pos + cmd_len > actual_len) {
dev_err_ratelimited(&dev->intf->dev,
"Format error\n");
break;
@@ -2156,6 +2157,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+ if (cmd_len < sizeof(struct kvaser_cmd)) {
+ dev_err(&dev->intf->dev, "Format error\n");
+ return;
+ }
+
if (pos + cmd_len > len) {
/* We got first part of a command */
int leftover_bytes;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 2/4] can: kvaser_usb: reject devices reporting zero max outstanding tx
2026-11-12 17:27 [PATCH v3 0/4] can: kvaser_usb: validate values received from the device Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 1/4] can: kvaser_usb: hydra: reject commands shorter than a standard command Joas Antonio dos Santos via B4 Relay
@ 2026-11-12 17:27 ` Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 3/4] can: kvaser_usb: hydra: do not read the command length before it is received Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 4/4] can: kvaser_usb: hydra: check the payload of extended rx commands Joas Antonio dos Santos via B4 Relay
3 siblings, 0 replies; 5+ messages in thread
From: Joas Antonio dos Santos via B4 Relay @ 2026-11-12 17:27 UTC (permalink / raw)
To: mkl, mailhol, joasantonio108; +Cc: linux-can, linux-kernel, stable
From: Joas Antonio dos Santos <joasantonio108@gmail.com>
The number of tx contexts is taken from the max_outstanding_tx field of
the software info reply sent by the device. The leaf driver uses the
value as is and the hydra driver only limits it to
KVASER_USB_MAX_TX_URBS, so a device can report zero.
dev->max_tx_urbs is then used as a divisor when a tx acknowledge is
received:
kvaser_usb_leaf_tx_acknowledge()
context = &priv->tx_contexts[tid % dev->max_tx_urbs];
kvaser_usb_hydra_tx_acknowledge()
context = &priv->tx_contexts[transid % dev->max_tx_urbs];
With max_tx_urbs == 0 a tx acknowledge sent by the device while the
interface is up is a division by zero in the URB completion handler on
architectures that trap on it. On architectures that do not
trap, the index is not reduced at all and tx_contexts[], which was
allocated with zero elements, is accessed out of bounds.
A device that cannot accept any outstanding tx frame is not usable, so
fail the probe when it reports zero.
Found by LLM-assisted source review.
Fixes: 082d70b6142f ("can: kvaser_usb: Comply with firmware max tx URBs value")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index d0a2a2a33c1c..a915e529290b 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -977,6 +977,12 @@ static int kvaser_usb_probe(struct usb_interface *intf,
goto free_devlink;
}
+ if (!dev->max_tx_urbs) {
+ dev_err(&intf->dev, "Invalid max outstanding tx (0)\n");
+ err = -ENODEV;
+ goto free_devlink;
+ }
+
if (ops->dev_get_software_details) {
err = ops->dev_get_software_details(dev);
if (err) {
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 3/4] can: kvaser_usb: hydra: do not read the command length before it is received
2026-11-12 17:27 [PATCH v3 0/4] can: kvaser_usb: validate values received from the device Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 1/4] can: kvaser_usb: hydra: reject commands shorter than a standard command Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 2/4] can: kvaser_usb: reject devices reporting zero max outstanding tx Joas Antonio dos Santos via B4 Relay
@ 2026-11-12 17:27 ` Joas Antonio dos Santos via B4 Relay
2026-11-12 17:27 ` [PATCH v3 4/4] can: kvaser_usb: hydra: check the payload of extended rx commands Joas Antonio dos Santos via B4 Relay
3 siblings, 0 replies; 5+ messages in thread
From: Joas Antonio dos Santos via B4 Relay @ 2026-11-12 17:27 UTC (permalink / raw)
To: mkl, mailhol, joasantonio108; +Cc: linux-can, linux-kernel, stable
From: Joas Antonio dos Santos <joasantonio108@gmail.com>
kvaser_usb_hydra_cmd_size() reads the 16 bit length of an extended
command from bytes 4 and 5 of the command. The two command parsers call
it without checking that those bytes have been received:
- kvaser_usb_hydra_wait_cmd() only requires pos < actual_len. If the
transfer fills the whole KVASER_USB_RX_BUFFER_SIZE buffer and the
last command starts in its last five bytes, the length is read from
beyond the end of the kzalloc()ed buffer.
- kvaser_usb_hydra_read_bulk_callback() has the same problem at the
end of the receive buffer. In addition, an extended command that is
split so that fewer than six bytes arrive in the first transfer is
parsed with a length taken from stale buffer content. That either
fails the length check or stores the fragment in usb_rx_leftover,
where the next callback computes the length again before the rest of
the header has been copied in, and without checking it against
sizeof(struct kvaser_cmd).
Every command is at least sizeof(struct kvaser_cmd) long and
kvaser_usb_hydra_wait_cmd() does not handle partial commands, so require
that many bytes there before looking at the command.
In kvaser_usb_hydra_read_bulk_callback(), buffer the fragment when the
length field is not complete, and complete the length field in
usb_rx_leftover before using it. Apply the minimum length check to the
buffered command too. Drop the buffered fragment when that check or
the existing check against the size of usb_rx_leftover fails; the
latter kept the fragment, so that the same error was hit again on the
following transfers.
Found by the Sashiko AI review of the previous version of this series.
Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 50 +++++++++++++++++++----
1 file changed, 43 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 9adb4eb36e39..6b68e0714ae3 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -675,6 +675,12 @@ static int kvaser_usb_hydra_wait_cmd(const struct kvaser_usb *dev, u8 cmd_no,
size_t cmd_len;
tmp_cmd = buf + pos;
+ if (actual_len - pos < sizeof(struct kvaser_cmd)) {
+ dev_err_ratelimited(&dev->intf->dev,
+ "Format error\n");
+ break;
+ }
+
cmd_len = kvaser_usb_hydra_cmd_size(tmp_cmd);
if (cmd_len < sizeof(struct kvaser_cmd) ||
pos + cmd_len > actual_len) {
@@ -2117,6 +2123,8 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
&dev->card_data.hydra;
int usb_rx_leftover_len;
spinlock_t *usb_rx_leftover_lock = &card_data->usb_rx_leftover_lock;
+ /* Number of bytes needed to tell the length of any command */
+ const int len_end = offsetofend(struct kvaser_cmd_ext, len);
spin_lock_irqsave(usb_rx_leftover_lock, irq_flags);
usb_rx_leftover_len = card_data->usb_rx_leftover_len;
@@ -2125,20 +2133,43 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
cmd = (struct kvaser_cmd *)card_data->usb_rx_leftover;
+ if (usb_rx_leftover_len < len_end) {
+ /* Get the rest of the length field first */
+ remaining_bytes = min(len,
+ len_end - usb_rx_leftover_len);
+ memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len,
+ buf, remaining_bytes);
+ pos += remaining_bytes;
+ usb_rx_leftover_len += remaining_bytes;
+ card_data->usb_rx_leftover_len = usb_rx_leftover_len;
+ if (usb_rx_leftover_len < len_end) {
+ spin_unlock_irqrestore(usb_rx_leftover_lock,
+ irq_flags);
+ return;
+ }
+ }
+
cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+ if (cmd_len < sizeof(struct kvaser_cmd)) {
+ 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,
+ 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,
- remaining_bytes);
+ 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) {
@@ -2155,11 +2186,16 @@ 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);
+ if (len - pos < len_end) {
+ /* The length is not known yet, buffer what we got */
+ cmd_len = sizeof(struct kvaser_cmd);
+ } else {
+ cmd_len = kvaser_usb_hydra_cmd_size(cmd);
- if (cmd_len < sizeof(struct kvaser_cmd)) {
- dev_err(&dev->intf->dev, "Format error\n");
- return;
+ if (cmd_len < sizeof(struct kvaser_cmd)) {
+ dev_err(&dev->intf->dev, "Format error\n");
+ return;
+ }
}
if (pos + cmd_len > len) {
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 4/4] can: kvaser_usb: hydra: check the payload of extended rx commands
2026-11-12 17:27 [PATCH v3 0/4] can: kvaser_usb: validate values received from the device Joas Antonio dos Santos via B4 Relay
` (2 preceding siblings ...)
2026-11-12 17:27 ` [PATCH v3 3/4] can: kvaser_usb: hydra: do not read the command length before it is received Joas Antonio dos Santos via B4 Relay
@ 2026-11-12 17:27 ` Joas Antonio dos Santos via B4 Relay
3 siblings, 0 replies; 5+ messages in thread
From: Joas Antonio dos Santos via B4 Relay @ 2026-11-12 17:27 UTC (permalink / raw)
To: mkl, mailhol, joasantonio108; +Cc: linux-can, linux-kernel, stable
From: Joas Antonio dos Santos <joasantonio108@gmail.com>
kvaser_usb_hydra_rx_msg_ext() reads the error frame data or copies up
to 64 bytes of CAN FD payload from an extended command. Both start at
byte 32 of the command, and the number of payload bytes is derived from
the DLC in kcan_header. Neither is checked against the length of the
command, which the parsers only guarantee to be at least
sizeof(struct kvaser_cmd), 32 bytes.
A device can therefore send a short CMD_RX_MESSAGE_FD command with a
large DLC and make the driver copy up to 64 bytes that follow the
command into the CAN frame that is passed to user space: stale data of
earlier transfers in the receive buffer or in usb_rx_leftover, or data
beyond the 3072 bytes of the receive buffer if the command is the last
one in a full transfer.
Check that the command contains the error frame data and the payload
before using them, and count the command as an rx error otherwise.
Found by the Sashiko AI review of the previous version of this series.
Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 15 +++++++++++++++
1 file changed, 15 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 6b68e0714ae3..387a522c60fa 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1308,6 +1308,7 @@ static void kvaser_usb_hydra_rx_msg_ext(const struct kvaser_usb *dev,
struct sk_buff *skb;
struct skb_shared_hwtstamps *shhwtstamps;
struct net_device_stats *stats;
+ size_t cmd_len = le16_to_cpu(cmd->len);
u32 flags;
u8 dlc;
u32 kcan_header;
@@ -1327,6 +1328,12 @@ static void kvaser_usb_hydra_rx_msg_ext(const struct kvaser_usb *dev,
hwtstamp = kvaser_usb_hydra_ktime_from_cmd(dev->cfg, std_cmd);
if (flags & KVASER_USB_HYDRA_CF_FLAG_ERROR_FRAME) {
+ if (cmd_len < offsetofend(struct kvaser_cmd_ext,
+ rx_can.err_frame_data)) {
+ stats->rx_errors++;
+ return;
+ }
+
kvaser_usb_hydra_error_frame(priv, &cmd->rx_can.err_frame_data,
hwtstamp);
return;
@@ -1370,6 +1377,14 @@ static void kvaser_usb_hydra_rx_msg_ext(const struct kvaser_usb *dev,
if (flags & KVASER_USB_HYDRA_CF_FLAG_REMOTE_FRAME) {
cf->can_id |= CAN_RTR_FLAG;
} else {
+ /* The payload must be part of the received command */
+ if (cmd_len < offsetof(struct kvaser_cmd_ext,
+ rx_can.kcan_payload) + cf->len) {
+ stats->rx_errors++;
+ kfree_skb(skb);
+ return;
+ }
+
memcpy(cf->data, cmd->rx_can.kcan_payload, cf->len);
stats->rx_bytes += cf->len;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread