[PATCH 0/2] can: esd_usb: validate data received from the device

[PATCH 0/2] can: esd_usb: validate data received from the device

[Joas Antonio dos Santos via B4 Relay]

Two fixes for values that esd_usb takes from the USB device without
checking them.

Patch 1 limits the number of nets reported in the version reply to
ESD_USB_MAX_NETS.  Marc Kleine-Budde asked for this clamp during the
review of the esd_usb cleanup series in August 2025; as far as I can
tell it never landed.

Patch 2 validates the message length in the rx path, so that a zero
length cannot stall the URB completion handler and a message is not
handled before checking that it fits in the received data.

These issues were found with the help of an LLM-based code review tool
(Claude).  I checked the code paths by hand against v7.4-rc3 and
reviewed the fixes myself.  They are build-tested only (W=1, arm64); I
do not have the hardware to test them at runtime.

---
Joas Antonio dos Santos (2):
      can: esd_usb: limit the number of nets reported by the device
      can: esd_usb: validate the message length in the rx path

 drivers/net/can/usb/esd_usb.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
---
base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
change-id: 20261110-esd-usb-validate-14264b90e5bb

Best regards,
-- 
Joas Antonio dos Santos <joasantonio108@gmail.com>

[PATCH 1/2] can: esd_usb: limit the number of nets reported by the device

[Joas Antonio dos Santos via B4 Relay]

From: Joas Antonio dos Santos <joasantonio108@gmail.com>

esd_usb_probe() takes the number of CAN nets from the version reply
sent by the device and uses it unchecked as the loop bound for
esd_usb_probe_one_net(), which stores the new private data pointer in
dev->nets[index].  The nets array only has ESD_USB_MAX_NETS (2)
entries, while the device controlled u8 can be up to 255.

A device that reports more than two nets makes the driver write
pointers past the end of the array, first over the following members
of struct esd_usb and then beyond the end of the allocation.  The
loops in esd_usb_read_bulk_callback(), unlink_all_urbs() and
esd_usb_disconnect() read dev->nets[] with the same bound.

A malicious USB device can use this to corrupt kernel heap memory
just by being plugged in.

Clamp the count to the size of the array.

Found by LLM-assisted source review.

Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
 drivers/net/can/usb/esd_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
index f41d4a0d140f..f26a8780686f 100644
--- a/drivers/net/can/usb/esd_usb.c
+++ b/drivers/net/can/usb/esd_usb.c
@@ -1343,7 +1343,7 @@ static int esd_usb_probe(struct usb_interface *intf,
 		goto free_msg;
 	}
 
-	dev->net_count = (int)msg->version_reply.nets;
+	dev->net_count = min_t(int, msg->version_reply.nets, ESD_USB_MAX_NETS);
 	dev->version = le32_to_cpu(msg->version_reply.version);
 
 	if (device_create_file(&intf->dev, &dev_attr_firmware))

-- 
2.43.0

[PATCH 2/2] can: esd_usb: validate the message length in the rx path

[Joas Antonio dos Santos via B4 Relay]

From: Joas Antonio dos Santos <joasantonio108@gmail.com>

esd_usb_read_bulk_callback() walks the messages in a received buffer
using the length byte from each message header:

	pos += msg->hdr.len * sizeof(u32); /* convert to # of bytes */

	if (pos > urb->actual_length) {

The length is supplied by the device and is not checked against zero.
A message with hdr.len == 0 leaves pos unchanged, so the loop never
terminates and keeps handling the same message in the URB completion
handler, which locks up the CPU.

The check that the declared length lies within the received data is
also done only after the message has been handled.

Compute the message length first and reject a message that is empty or
whose declared length does not fit into the remaining data before it is
handled.  The handlers still rely on the fixed layout of the message
they are given; checking the declared length against the layout of each
command is left for a separate change.

Found by LLM-assisted source review.

Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
 drivers/net/can/usb/esd_usb.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
index f26a8780686f..67dba118ce9e 100644
--- a/drivers/net/can/usb/esd_usb.c
+++ b/drivers/net/can/usb/esd_usb.c
@@ -507,8 +507,15 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 
 	while (pos < urb->actual_length) {
 		union esd_usb_msg *msg;
+		unsigned int msg_len;
 
 		msg = (union esd_usb_msg *)(urb->transfer_buffer + pos);
+		msg_len = msg->hdr.len * sizeof(u32); /* convert to # of bytes */
+
+		if (!msg_len || msg_len > urb->actual_length - pos) {
+			dev_err(dev->udev->dev.parent, "format error\n");
+			break;
+		}
 
 		switch (msg->hdr.cmd) {
 		case ESD_USB_CMD_CAN_RX:
@@ -531,12 +538,7 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 			break;
 		}
 
-		pos += msg->hdr.len * sizeof(u32); /* convert to # of bytes */
-
-		if (pos > urb->actual_length) {
-			dev_err(dev->udev->dev.parent, "format error\n");
-			break;
-		}
+		pos += msg_len;
 	}
 
 resubmit_urb:

-- 
2.43.0