On 05/07/18(Thu) 09:28, Landry Breuil wrote:
> On Fri, Jun 29, 2018 at 01:50:24PM +0200, Landry Breuil wrote:
> > On Fri, Jun 29, 2018 at 09:05:56AM +0200, Remco wrote:
> > > > > If you don't find any better solution, I'd suggest using a device ID
> > > > > check rather than adding a quirk. Because such quirk cannot be
> > > > > generic.
> > > > > In that case you have an off-by-one, but another device might have a
> > > > > different problem.
> > > >
> > > > That was in the case we'd encounter other devices where the descriptor
> > > > is bogus and has the same issue, but i agree this is unlikely.
> > > >
> > >
> > > More likely for Logitech USB webcams I'm afraid.
> > > My dad's Logitech C500 showed the same issue several years back.
> > > (I'll try to find some time to borrow the device and test your patch)
> > > There's also a report that suggests a C200 may be affected as well.
> > > (https://marc.info/?l=openbsd-misc&m=127952275021290&w=2)
> >
> > Aaah, very interesting data point. Thanks for the pointer.. maybe a
> > 'generic fix' would be to ignore size checks for all logitech webcams,
> > or to print a warning but not error out in this case. Is there some kind
> > of 'usbdevs -v/lsusb -v' database somewhere ?
>
> So after feedback from lots of logitech webcam users (thanks!), i got a list
> for working vs non-working devices, and i think a new quirk would do as they
> all expose the same issue (bLength for FEATURE_UNIT is 9 while it is 8 on
> working devices) - and they would all be 'fixed' by the attached diff.
If you know that some descriptors have an off-by-one, I'd suggest you add
1 to the corresponding buffer length instead of completely removing the
check. Removing it completely now open the door to other overflows and
adding 1 auto-document the hardware bug ;)
> If an overflow is detected on the descriptor size (ie the (ibuf +
> dp->bLength > ibufend) check but we know we're on a broken device (with
> UAUDIO_FLAG_BAD_ADC_LEN quirk) we silently avoid returning USBD_INVAL,
> and that allows uaudio(4) detection to succeed. That's a different take than
> adding 1 to aclen in the first version of the diff, i dunno which is better.
>
> Now, i'm pretty sure i managed to properly test the device some days ago
> and record sound with it, but now i only manage to produce 68-bytes
> empty wav files, using 'aucat -f rsnd/1 -o /tmp/test.wav' after having
> ensured kern.audio.record was 1. Something is fishy....
>
> Oks, testing & feedback welcome, so that i can move forward and get more
> testing/debugging done.
>
> Landry
>
> Index: uaudio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.128
> diff -u -r1.128 uaudio.c
> --- uaudio.c 30 Dec 2017 23:08:29 -0000 1.128
> +++ uaudio.c 3 Jul 2018 06:44:28 -0000
> @@ -172,6 +172,7 @@
> #define UAUDIO_FLAG_VENDOR_CLASS 0x0010 /* claims vendor class but
> works */
> #define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must
> equal */
> #define UAUDIO_FLAG_EMU0202 0x0040
> +#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor
> size */
>
> struct uaudio_devs {
> struct usb_devno uv_dev;
> @@ -222,6 +223,16 @@
> UAUDIO_FLAG_BAD_AUDIO },
> { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
> UAUDIO_FLAG_BAD_AUDIO },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC200 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC210 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC270 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> + { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC500 },
> + UAUDIO_FLAG_BAD_ADC_LEN },
> { { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
> UAUDIO_FLAG_NO_FRAC }
> };
> @@ -1832,7 +1843,8 @@
> if (ibuf >= ibufend)
> break;
> dp = (const usb_descriptor_t *)ibuf;
> - if (ibuf + dp->bLength > ibufend) {
> + if (ibuf + dp->bLength > ibufend &&
> + !(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)) {
> free(iot, M_TEMP, 0);
> return (USBD_INVAL);
> }
>
