From: Joas Antonio dos Santos 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 --- 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 From: Joas Antonio dos Santos 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 --- 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