On Thu, Jan 28, 2021 at 02:38:04PM +0000, Edd Barrett wrote:
> On Thu, Jan 28, 2021 at 09:56:14AM +0000, Edd Barrett wrote:
> >
>
> Here's a revised diff that always searches the array, instead of first
> trying the expected index. Everyone agreed that this makes for simpler
> code, and that since this function isn't called much, there's no real
> performance concern.
>
> Cursory testing seems good. All USB devices working so far, including
> the non-compliant uaudio device.
>
> Please give this a spin to check for breakage.
The code looks good to me (cleaner than before), and I also quickly
tested the diff with some devices.
ok mglocker@
> Index: usbdi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 usbdi.c
> --- usbdi.c 27 Jan 2021 17:28:19 -0000 1.107
> +++ usbdi.c 28 Jan 2021 14:19:28 -0000
> @@ -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 */
