From: edd <edd@openbsd.org>
Date: Fri, 29 Jan 2021 11:44:06 +0000 (+0000)
Subject: Don't rely on USB interfaces being at compliant indices.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=3882cb129944bae6f8b229f8862dcf37c6bc2873;p=openbsd

Don't rely on USB interfaces being at compliant indices.

When obtaining an interface handle, we currently rely on the device being
properly USB compliant, and thus the interface being at the correct index in
the interfaces array.

However, some devices present their indices incorrectly. For example, the
following audio device exposes interfaces 0, 1 and 3, in that order (skipping
interface 2 entirely):

uaudio2 at uhub4 port 4 configuration 1 interface 3 "E+ Corp. DAC Audio" rev 1.10/0.01 addr 2
uaudio2: class v1, full-speed, async, channels: 2 play, 0 rec, 3 ctls

This means that that the audio stream interface (number 3) is not found at the
expected index of 2, and this causes looking up the handle to fail.

This change makes usbd_device2interface_handle() search for the right
interface, instead of assuming it will be at the right index. Although this is
a little slower, note that this routine not very frequently called and there
are typically not hundreds of interfaces on a typical USB device.

This fixes the above E+ Corp device, and one other uaudio device reported
broken by a user.

With input from, tested by, and OK ratchov@, mglocker@ and kettenis@.

Many thanks!
---

diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index a6be95a040a..cf8bad76d44 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usbdi.c,v 1.107 2021/01/27 17:28:19 mglocker Exp $ */
+/*	$OpenBSD: usbdi.c,v 1.108 2021/01/29 11:44:06 edd Exp $ */
 /*	$NetBSD: usbdi.c,v 1.103 2002/09/27 15:37:38 provos Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usbdi.c,v 1.28 1999/11/17 22:33:49 n_hibma Exp $	*/
 
@@ -638,12 +638,23 @@ usbd_status
 usbd_device2interface_handle(struct usbd_device *dev, u_int8_t ifaceno,
     struct usbd_interface **iface)
 {
+	u_int8_t idx;
+
 	if (dev->cdesc == NULL)
 		return (USBD_NOT_CONFIGURED);
-	if (ifaceno >= dev->cdesc->bNumInterfaces)
-		return (USBD_INVAL);
-	*iface = &dev->ifaces[ifaceno];
-	return (USBD_NORMAL_COMPLETION);
+	/*
+	 * The correct interface should be at dev->ifaces[ifaceno], but we've
+	 * seen non-compliant devices in the wild which present non-contiguous
+	 * interface numbers and this skews the indices. For this reason we
+	 * linearly search the interface array.
+	 */
+	for (idx = 0; idx < dev->cdesc->bNumInterfaces; idx++) {
+		if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
+			*iface = &dev->ifaces[idx];
+			return (USBD_NORMAL_COMPLETION);
+		}
+	}
+	return (USBD_INVAL);
 }
 
 /* XXXX use altno */