Skip to contentSkip to conversation

USB: usbtmc: add missing endpoint sanity check

PATCH New Johan Hovold 1 participant 2 messages Mar 14, 2017

Subsystems (2)
Stable branch USB
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

PATCH 1/2 USB: usbtmc: add missing endpoint sanity check

USBTMC devices are required to have a bulk-in and a bulk-out endpoint, but the driver failed to verify this, something which could lead to the endpoint addresses being taken from uninitialised memory.

Make sure to zero all private data as part of allocation, and add the missing endpoint sanity check.

Note that this also addresses a more recently introduced issue, where the interrupt-in-presence flag would also be uninitialised whenever the optional interrupt-in endpoint is not present. This in turn could lead to an interrupt urb being allocated, initialised and submitted based on uninitialised values.

Fixes dbf3e7f654c0 ("Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation.")
Fixes 5b775f672cc9 ("USB: add USB test and measurement class driver")
Cc stable <[email protected]> # 2.6.28
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/class/usbtmc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/usb/class/usbtmc.c +9 -1
index f03692ec5520..5e3446db4513 100644
@@ -1381,7 +1381,7 @@ static int usbtmc_probe(struct usb_interface *intf,
 
 	dev_dbg(&intf->dev, "%s called\n", __func__);
 
-	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 

@@ -1444,6 +1444,13 @@ static int usbtmc_probe(struct usb_interface *intf,
 			break;
 		}
 	}
+
+	if (!data->bulk_out || !data->bulk_in) {
+		dev_err(&intf->dev, "bulk endpoints not found\n");
+		retcode = -ENODEV;
+		goto err_put;
+	}
+
 	/* Find int endpoint */
 	for (n = 0; n < iface_desc->desc.bNumEndpoints; n++) {
 		endpoint = &iface_desc->endpoint[n].desc;

@@ -1512,6 +1519,7 @@ static int usbtmc_probe(struct usb_interface *intf,
 	sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
 	sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
 	usbtmc_free_int(data);
+err_put:
 	kref_put(&data->kref, usbtmc_delete);
 	return retcode;
 }
Signature · 1 line
2.12.0
author
#1 lore raw

PATCH 2/2 USB: usbtmc: fix probe error path

Make sure to initialise the return value to avoid having allocation failures going unnoticed when allocating interrupt-endpoint resources.

This prevents use-after-free or worse when the device is later unbound.

Fixes dbf3e7f654c0 ("Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation.")
Cc stable <[email protected]> # 4.6
Cc Dave Penkler <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/class/usbtmc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/usb/class/usbtmc.c +6 -2
index 5e3446db4513..8fb309a0ff6b 100644
@@ -1476,8 +1476,10 @@ static int usbtmc_probe(struct usb_interface *intf,
 	if (data->iin_ep_present) {
 		/* allocate int urb */
 		data->iin_urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!data->iin_urb)
+		if (!data->iin_urb) {
+			retcode = -ENOMEM;
 			goto error_register;
+		}
 
 		/* Protect interrupt in endpoint data until iin_urb is freed */
 		kref_get(&data->kref);

@@ -1485,8 +1487,10 @@ static int usbtmc_probe(struct usb_interface *intf,
 		/* allocate buffer for interrupt in */
 		data->iin_buffer = kmalloc(data->iin_wMaxPacketSize,
 					GFP_KERNEL);
-		if (!data->iin_buffer)
+		if (!data->iin_buffer) {
+			retcode = -ENOMEM;
 			goto error_register;
+		}
 
 		/* fill interrupt urb */
 		usb_fill_int_urb(data->iin_urb, data->usb_dev,
Signature · 1 line
2.12.0