On Tue, Jan 26, 2021 at 08:56:29PM +0000, Edd Barrett wrote:
> Hi,
>
> I've recently come across a uaudio device which doesn't work in OpenBSD:
>
> uaudio2 at uhub0 port 1 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
> audio3 at uaudio2
>
> When opening the audio device, the kernel reports `uaudio0: can't get iface
> handle` in the dmesg buffer.
>
> I posted information about the device and sthen@ and kettenis@ spotted
> the problem: the device exposes non-consecutive interface numbers, and
> is thus not compliant with the USB standard.
>
> It has interfaces 0, 1 and 3, in that order. The audio stream is on
> interface 3, which the kernel expects to find at index 3 in the ifaces
> array, but actually it's at index 2!
>
> I've pasted a load of info about the device (lsusb and debug kernel
> output) here:
> https://gist.github.com/vext01/958756d0fd515cc321f99a3ee1e3351a
>
> The following diff makes this device work. It searches the ifaces array
> in the event that the correct entry is not found at the expect index.
> This should be a "no functional change" for all existing compliant
> devices.
>
> I've tested this for the past two days and all seems well, but I'd like
> some USB stack hackers to verify it.
I'm OK with this approach. Also I can't currently see how that would
break other devices. I gave it a quick spin here and couldn't find any
issues.
If you could wrap the comment to 80 chars would be nice. Otherwise
ok mglocker@
BTW:
The first thing which jumped in my face when reading your diff is that
the configuration descriptor member 'bNumInterface' should be called
'bNumInterfaces' instead, but I'll send out a separate diff for that
later :-)
> Cheers
>
>
> Index: usbdi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 usbdi.c
> --- usbdi.c 3 Apr 2020 20:11:47 -0000 1.106
> +++ usbdi.c 24 Jan 2021 20:44: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->bNumInterface)
> - return (USBD_INVAL);
> - *iface = &dev->ifaces[ifaceno];
> - return (USBD_NORMAL_COMPLETION);
> + if (ifaceno < dev->cdesc->bNumInterface &&
> + dev->ifaces[ifaceno].idesc->bInterfaceNumber == ifaceno) {
> + *iface = &dev->ifaces[ifaceno];
> + return (USBD_NORMAL_COMPLETION);
> + }
> + /* With some non-compliant devices, the interface may be at the wrong
> index */
> + for (idx = 0; idx < dev->cdesc->bNumInterface; idx++) {
> + if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
> + *iface = &dev->ifaces[idx];
> + return (USBD_NORMAL_COMPLETION);
> + }
> + }
> + return (USBD_INVAL);
> }
>
> /* XXXX use altno */
