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 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);
}
