Thread (18 messages) 18 messages, 4 authors, 2026-03-05

Re: [PATCH net 05/12] can: ems_usb: ems_usb_read_bulk_callback(): check the proper length of a message

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2026-03-03 14:34:16
Also in: linux-can

On 03.03.2026 15:20:01, Paolo Abeni wrote:
AI noticed the following:
---
Does the check validate enough of the message? The code checks that the
11-byte header (CPC_MSG_HEADER_LEN) fits in the buffer, but
ems_usb_rx_can_msg() and ems_usb_rx_err() both access fields in the
msg->msg union payload starting at offset 11.

For example, ems_usb_rx_can_msg() reads:
- msg->msg.can_msg.id (4 bytes at offset 11)
- msg->msg.can_msg.length (1 byte at offset 15)
- msg->msg.can_msg.msg[i] (up to 8 bytes at offset 16)

Similarly, ems_usb_rx_err() reads:
- msg->msg.can_state (1 byte at offset 11)
- msg->msg.error.cc.regs.sja1000.{ecc,txerr,rxerr} (3 bytes at offset 11+)

A malicious USB device could send a packet where actual_length equals
CPC_HEADER_SIZE + CPC_MSG_HEADER_LEN (15 bytes total), which would pass
this check but provide zero payload bytes. The callees would then read
beyond the received buffer before reaching the second check at the end
of the loop.

Should the validation check that both the header and payload
(msg->length bytes) fit within actual_length before calling the
processing functions?
---
I guess this patch could need a follow-up? Not blocking the PR, as
AFAICS worst case this patch still improves the current situation.
Yes, Greg said he'll look into this

| https://lore.kernel.org/all/2026030253-lather-pulse-3bb5@gregkh/ (local)

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help