[PATCH v3 0/2] usb: usbtmc: add sanity checks for interrupt endpoints

[PATCH v3 0/2] usb: usbtmc: add sanity checks for interrupt endpoints

[Heitor Alves de Siqueira]

Dear maintainers,

This is a v3 patch for an out-of-bounds read originally reported by
syzbot in [0]. After discussion, I've split the original fix into two
separate patches.

The first patch is a more conservative check against each URB in the
usbtmc_interrupt() path, to ensure enough data was transferred to
include the necessary payload headers. I've tagged this one for stable,
as it shouldn't impact existing devices besides fixing the out-of-bounds
read. Devices that eventually raise problematic interrupt notifications
will be able to try again once the URB is resubmitted.

The second patch is more strict, as it rejects devices that advertise
interrupt endpoints with wMaxPacketSize below 2 bytes. This approach was
suggested during v1 of this series, as these devices are unlikely to
exist and won't work properly with the current usbtmc driver. This
approach is also more aligned with the intent of the USB488 spec, as
interrupt endpoints should ideally be setup with enough space for
the payload headers.
While the first patch is sufficient to fix the out-of-bounds read, there
seems to be little point in having those interrupt endpoints configured
if the driver will ignore all URBs from it.

[0] https://syzkaller.appspot.com/bug?extid=abbfd103085885cf16a2

---
Changes in v3:
- Split into two patches:
  - actual_length check in usbtmc_interrupt() for the syzbot fix
  - wMaxPacketSize check in usbtmc_probe() to reject quirky devices
- Link to v2: https://patch.msgid.link/20260423-usbtmc-iin-size-v2-1-31afa4874f71@igalia.com

Changes in v2:
- Instead of ensuring buffer size, reject devices that advertise illegal/invalid interrupt endpoints
- Link to v1: https://patch.msgid.link/20260422-usbtmc-iin-size-v1-1-5dc44b4389aa@igalia.com

---
Heitor Alves de Siqueira (2):
      usb: usbtmc: check URB actual_length for interrupt-IN notifications
      usb: usbtmc: reject interrupt endpoints with small wMaxPacketSize

 drivers/usb/class/usbtmc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
---
base-commit: 70c8a7ec6715b5fb14e501731b5b9210a16684f7
change-id: 20260422-usbtmc-iin-size-f1aaf04a6c4c

Best regards,
--  
Heitor Alves de Siqueira <halves@igalia.com>

[PATCH v3 1/2] usb: usbtmc: check URB actual_length for interrupt-IN notifications

[Heitor Alves de Siqueira]

USBTMC devices can use an optional interrupt endpoint for notification
messages. These typically contain two-byte headers indicating the
payload format, but the driver does not check if these headers are
present before accessing the data buffers. In cases where the URB
actual_length is not enough to fit these headers, the driver will either
cause an out-of-bounds read, or consume stale leftover data from a
previous notification.

Fix by checking if actual_data contains enough bytes for the headers,
otherwise resubmit URB to the interrupt endpoint.

Fixes: dbf3e7f654c0 ("Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation.")
Reported-by: syzbot+abbfd103085885cf16a2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=abbfd103085885cf16a2
Cc: stable@kernel.org
Suggested-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Heitor Alves de Siqueira <halves@igalia.com>
---
 drivers/usb/class/usbtmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index bd9347804dec..e15efd0c5ca7 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -2306,6 +2306,14 @@ static void usbtmc_interrupt(struct urb *urb)
 
 	switch (status) {
 	case 0: /* SUCCESS */
+		/* ensure at least two bytes of headers were transferred */
+		if (urb->actual_length < 2) {
+			dev_warn(dev,
+				"actual length %d not sufficient for interrupt headers\n",
+				urb->actual_length);
+			goto exit;
+		}
+
 		/* check for valid STB notification */
 		if (data->iin_buffer[0] > 0x81) {
 			data->bNotify1 = data->iin_buffer[0];

-- 
2.54.0

Re: [PATCH v3 1/2] usb: usbtmc: check URB actual_length for interrupt-IN notifications

[Alan Stern]

On Tue, May 05, 2026 at 03:56:03PM -0300, Heitor Alves de Siqueira wrote:
> USBTMC devices can use an optional interrupt endpoint for notification
> messages. These typically contain two-byte headers indicating the
> payload format, but the driver does not check if these headers are
> present before accessing the data buffers. In cases where the URB
> actual_length is not enough to fit these headers, the driver will either
> cause an out-of-bounds read, or consume stale leftover data from a
> previous notification.
> 
> Fix by checking if actual_data contains enough bytes for the headers,
> otherwise resubmit URB to the interrupt endpoint.

Would it be simpler to solve this by setting the two header bytes to 0 
before submitting the URB?  Then if the device did not send enough data, 
the header values would be 0, which should prevent any reads from being 
out-of-bounds or getting stale data.

Alan Stern

Re: [PATCH v3 1/2] usb: usbtmc: check URB actual_length for interrupt-IN notifications

[Michal Pecio]

On Tue, 5 May 2026 15:17:59 -0400, Alan Stern wrote:
> > Fix by checking if actual_data contains enough bytes for the headers,
> > otherwise resubmit URB to the interrupt endpoint.  
> 
> Would it be simpler to solve this by setting the two header bytes to 0 
> before submitting the URB?  Then if the device did not send enough data, 
> the header values would be 0, which should prevent any reads from being 
> out-of-bounds or getting stale data.

This amounts to saying that:
1. for 0 byte packets, the default notification type is zero
2. for 1 byte packets, the parameter byte defaults to zero

which would result in:
1. a warning (similar to this patch)
2. who knows what, why even worry about such things? ;)


If anything, I think the new warning isn't truly necessary and it
is misleading, if vendor specific notifications with zero-length
bNotify2 field are legal. It would suffice to check actual_length
before accepting the particular 2-byte long notification types,
and leave shorter packets unhandled, causing the default warning.

Another missing bit of pedantry is that these types should only be
interpreted this way on bInterfaceProtocol == 1 devices. But as
long as there are no other protocols defined (are there?), no valid
device is allowed to use them for anything else.

Regards,
Michal

[PATCH v3 2/2] usb: usbtmc: reject interrupt endpoints with small wMaxPacketSize

[Heitor Alves de Siqueira]

The USB488 subclass specification requires interrupt wMaxPacketSize to
be 0x02, unless the device sends vendor-specific notifications.
Endpoints that advertise less than 2 bytes for wMaxPacketSize are
unlikely to work with the current driver, as URBs will not have enough
space for interrupt headers. Considering that any notification URBs will
be ignored by the driver, reject these endpoints early during probe.

Fixes: 041370cce889 ("USB: usbtmc: refactor endpoint retrieval")
Suggested-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Heitor Alves de Siqueira <halves@igalia.com>
---
 drivers/usb/class/usbtmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index e15efd0c5ca7..af9ae55dae14 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -2440,6 +2440,12 @@ static int usbtmc_probe(struct usb_interface *intf,
 		data->iin_ep = int_in->bEndpointAddress;
 		data->iin_wMaxPacketSize = usb_endpoint_maxp(int_in);
 		data->iin_interval = int_in->bInterval;
+		/* wMaxPacketSize should be 0x02 or more as per USB488 Table 22 */
+		if (iface_desc->desc.bInterfaceProtocol == 1 &&
+		    data->iin_wMaxPacketSize < 2) {
+			retcode = -EINVAL;
+			goto err_put;
+		}
 		dev_dbg(&intf->dev, "Found Int in endpoint at %u\n",
 				data->iin_ep);
 	}

-- 
2.54.0