On 2021/02/03 11:10, Edd Barrett wrote:
> 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.
btw the problem was seen with umb, it's not just ugen.
> 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.
At some point you just have to say "this device is broken crap, send
it back or ebay it and buy an alternative". This is much easier for some
classes of device where there are many alternatives (like audio interfaces)
than mobile broadband where it's still very difficult to find something
suitable with the correct physical interface.
> 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
