On Thu, Jul 05, 2018 at 11:04:34AM +0200, Martin Pieuchot wrote:
> 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 ;)
Well that was the intent of the original diff. New version...
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 5 Jul 2018 09:12:42 -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 }
};
@@ -1814,6 +1825,10 @@
sc->sc_audio_rev = UGETW(acdp->bcdADC);
DPRINTFN(2,("%s: found AC header, vers=%03x, len=%d\n",
__func__, sc->sc_audio_rev, aclen));
+
+ /* Some webcams descriptors advertise an off-by-one wTotalLength */
+ if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+ aclen++;
sc->sc_nullalt = -1;
