[PATCH 0/2] can: usb: validate URB length in PEAK-USB rx callbacks

[PATCH 0/2] can: usb: validate URB length in PEAK-USB rx callbacks

[Berkant Koc]

This series mirrors the two gs_usb hardening fixes that landed in 6.18
(commits 6fe9f3279f7d and 395d988f9386) for the two PEAK-System USB
drivers that share the same "walk records inside the bulk-in URB"
pattern: pcan_usb_fd.c and pcan_usb_pro.c. Both decode loops read the
on-wire record header before validating that the URB actually contains
that header, allowing a malicious USB device that emulates a PEAK CAN
adapter to trigger a short read of one or two bytes past the URB
buffer on every poll cycle.

Patch 1 adds a sizeof(struct pucan_msg) check at the top of the
pcan_usb_fd_decode_buf() loop and rejects records whose announced size
is smaller than the header itself.

Patch 2 adds a one-byte check before reading pr->data_type in
pcan_usb_pro_decode_buf(), which is the field used to index the
record-size table.

Both fixes are static-analysis-identified mirrors of the gs_usb
precedent. I did not have access to a PCAN-USB-FD or PCAN-USB-Pro
adapter and therefore no live KASAN trip is attached, but the trigger
condition (URB actual_length below the per-record header size) is
identical to the gs_usb case that Marc Kleine-Budde fixed in November.

A third candidate file, drivers/net/can/usb/usb_8dev.c, was reviewed
for the same pattern but already validates "pos + sizeof(struct
usb_8dev_rx_msg) > urb->actual_length" at the top of its loop and uses
a fixed-size record, so no patch is included for it.

Note: a concurrent series by James Gao (msgid <TYCPR01MB856782BAA657447E5EDDDC1FF0062>) hardens different sites in pcan_usb_pro.c (handle_canmsg + handle_error); the hunks in patch 2/2 here do not overlap.

Berkant Koc (2):
  can: peak_usb: validate URB length in pcan_usb_fd_decode_buf()
  can: peak_usb: validate URB length in pcan_usb_pro_decode_buf()

 drivers/net/can/usb/peak_usb/pcan_usb_fd.c  |  7 ++++---
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 15 +++++++++++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.47.3

[PATCH 1/2] can: peak_usb: validate URB length in pcan_usb_fd_decode_buf()

[Berkant Koc]

pcan_usb_fd_decode_buf() walks records inside the bulk-in URB by reading
the 12-byte struct pucan_msg header from the front of each record. The
existing loop only verifies that msg_ptr is below msg_end before
dereferencing rx_msg->size and rx_msg->type, which means a short URB
that contains between 1 and 11 bytes of payload causes a two-byte
out-of-bounds read of the rx_msg->size and rx_msg->type fields. The
fragment check that follows compares the announced size against msg_end
but lands after the header has already been read.

A malicious USB device that pretends to be a PEAK-System PCAN-USB-FD
adapter (USB IDs 0c72:0012, 0c72:0014, 0c72:0016) can keep returning
short bulk-in URBs and trigger this read on every poll cycle, leaking
adjacent slab content via the dispatched decode paths or simply
producing a KASAN slab-out-of-bounds report.

Apply the pattern from commit 6fe9f3279f7d ("can: gs_usb: gs_usb_receive_bulk_callback(): check actual_length before accessing header"):
require that at least sizeof(struct pucan_msg) bytes remain before each
iteration, and reject records whose announced size is smaller than the
header itself.

Identified by static analysis. No KASAN trip available without specific
PEAK CAN-FD hardware.

Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Cc: stable@vger.kernel.org # 4.0+
Signed-off-by: Berkant Koc <me@berkoc.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index eb4f5884ad73..63d93f90165c 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -726,7 +726,7 @@ static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 	/* loop reading all the records from the incoming message */
 	msg_ptr = urb->transfer_buffer;
 	msg_end = urb->transfer_buffer + urb->actual_length;
-	for (; msg_ptr < msg_end;) {
+	while (msg_ptr + sizeof(struct pucan_msg) <= msg_end) {
 		u16 rx_msg_type, rx_msg_size;
 
 		rx_msg = (struct pucan_msg *)msg_ptr;
@@ -738,8 +738,9 @@ static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 		rx_msg_size = le16_to_cpu(rx_msg->size);
 		rx_msg_type = le16_to_cpu(rx_msg->type);
 
-		/* check if the record goes out of current packet */
-		if (msg_ptr + rx_msg_size > msg_end) {
+		/* check if the record fits inside the current packet */
+		if (rx_msg_size < sizeof(struct pucan_msg) ||
+		    msg_ptr + rx_msg_size > msg_end) {
 			netdev_err(netdev,
 				   "got frag rec: should inc usb rx buf sze\n");
 			err = -EBADMSG;
-- 
2.47.3

Re: [PATCH 1/2] can: peak_usb: validate URB length in pcan_usb_fd_decode_buf()

[Vincent Mailhol]

On 17/05/2026 at 15:55, Berkant Koc wrote:
> pcan_usb_fd_decode_buf() walks records inside the bulk-in URB by reading
> the 12-byte struct pucan_msg header from the front of each record. The
> existing loop only verifies that msg_ptr is below msg_end before
> dereferencing rx_msg->size and rx_msg->type, which means a short URB
> that contains between 1 and 11 bytes of payload causes a two-byte
> out-of-bounds read of the rx_msg->size and rx_msg->type fields. The
> fragment check that follows compares the announced size against msg_end
> but lands after the header has already been read.
> 
> A malicious USB device that pretends to be a PEAK-System PCAN-USB-FD
> adapter (USB IDs 0c72:0012, 0c72:0014, 0c72:0016) can keep returning
> short bulk-in URBs and trigger this read on every poll cycle, leaking
> adjacent slab content via the dispatched decode paths or simply
> producing a KASAN slab-out-of-bounds report.
> 
> Apply the pattern from commit 6fe9f3279f7d ("can: gs_usb: gs_usb_receive_bulk_callback(): check actual_length before accessing header"):
> require that at least sizeof(struct pucan_msg) bytes remain before each
> iteration, and reject records whose announced size is smaller than the
> header itself.
> 
> Identified by static analysis.

Could you name which tool you used? Otherwise, this sentence adds little
value to the report.

> No KASAN trip available without specific PEAK CAN-FD hardware.
> 
> Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
> Cc: stable@vger.kernel.org # 4.0+
> Signed-off-by: Berkant Koc <me@berkoc.com>

The code itself is OK. Not withstanding of above comment:

Reviewed-by: Vincent Mailhol <mailhol@kernel.org>


Yours sincerely,
Vincent Mailhol

[PATCH 2/2] can: peak_usb: validate URB length in pcan_usb_pro_decode_buf()

[Berkant Koc]

pcan_usb_pro_decode_buf() walks variable-sized records inside the
bulk-in URB. Each iteration reads pr->data_type (the first byte of the
record) and indexes pcan_usb_pro_sizeof_rec[] with it before checking
that the announced record length fits within msg_end. When the URB
length exactly equals PCAN_USBPRO_MSG_HEADER_LEN (4 bytes),
pcan_msg_init() returns a rec_ptr that already equals msg_end while the
on-wire rec_cnt field can still be non-zero. The first iteration then
reads pr->data_type one byte past the URB buffer.

A malicious USB device that pretends to be a PEAK-System PCAN-USB-Pro
adapter (USB IDs 0c72:000c..0c72:0014 across the Pro and Pro-FD line)
can keep returning short bulk-in URBs and trigger this read on every
poll cycle, leaking one byte of adjacent slab content into the
dispatch table lookup or producing a KASAN slab-out-of-bounds report.

Apply the pattern from commit 6fe9f3279f7d ("can: gs_usb: gs_usb_receive_bulk_callback(): check actual_length before accessing header"):
verify that at least one byte remains before dereferencing the record
header.

Identified by static analysis. No KASAN trip available without specific
PEAK CAN-USB-Pro hardware.

Fixes: d8a199355f8f ("can: usb: PEAK-System Technik PCAN-USB Pro specific part")
Cc: stable@vger.kernel.org # 3.4+
Signed-off-by: Berkant Koc <me@berkoc.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 4bfa8d0fbb32..f7b0304f7c37 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -715,8 +715,19 @@ static int pcan_usb_pro_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 	msg_end = urb->transfer_buffer + urb->actual_length;
 	rec_cnt = le16_to_cpu(*usb_msg.u.rec_cnt_rd);
 	for (; rec_cnt > 0; rec_cnt--) {
-		union pcan_usb_pro_rec *pr = (union pcan_usb_pro_rec *)rec_ptr;
-		u16 sizeof_rec = pcan_usb_pro_sizeof_rec[pr->data_type];
+		union pcan_usb_pro_rec *pr;
+		u16 sizeof_rec;
+
+		/* make sure data_type is readable before dereferencing it */
+		if (rec_ptr >= msg_end) {
+			netdev_err(netdev,
+				   "got frag rec: should inc usb rx buf size\n");
+			err = -EBADMSG;
+			break;
+		}
+
+		pr = (union pcan_usb_pro_rec *)rec_ptr;
+		sizeof_rec = pcan_usb_pro_sizeof_rec[pr->data_type];
 
 		if (!sizeof_rec) {
 			netdev_err(netdev,
-- 
2.47.3