Hi,
CCing ratchov@ and kettenis@ with some context.
In short: my change broke ugen, which expects to scan up the interface
range until an interface doesn't exist.
On Wed, Feb 03, 2021 at 06:25:48AM +0100, Marcus Glocker wrote:
>
> Index: dev/usb/usbdi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.109
> diff -u -p -u -p -r1.109 usbdi.c
> --- dev/usb/usbdi.c 1 Feb 2021 09:21:51 -0000 1.109
> +++ dev/usb/usbdi.c 2 Feb 2021 06:07:41 -0000
> @@ -642,6 +642,10 @@ usbd_device2interface_handle(struct usbd
>
> if (dev->cdesc == NULL)
> return (USBD_NOT_CONFIGURED);
> + if (ifaceno < dev->cdesc->bNumInterfaces) {
> + *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
>
> So OK if I commit this fix Edd, Stuart?
I'm OK with it as a quick-fix. At least it will make both of the devices
in question work.
But in the long run, it's not hard to imagine other non-compliant
devices which would still be defeated by this code.
Suppose a device presents its contiguous interfaces in reverse order, e.g.:
[2, 1, 0]. Now suppose a device driver asks for interface 2. We will
find interface 0, as we never check if it's the right interface and we
never reach the part of the code that scans the array.
In other words, just because an index exists, doesn't mean it's the right
interface.
I think (and I'm not much of a kernel hacker, so I reserve the right be wrong)
the correct solution is to:
* always loop over the array looking for the right interface.
* change ugen, so that it's scanning resilient to gaps in interface range.
--
Best Regards
Edd Barrett
http://www.theunixzoo.co.uk
