On Fri, Jul 06, 2018 at 03:38:00PM +0200, Remco wrote:
> > @@ -1814,6 +1827,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;
> >
> >
>
> I'm not in a position to test yet, but I have a comment on the aclen usage.
> AFAICS aclen is used in two places with the fix in between:
>
> aclen = UGETW(acdp->wTotalLength);
> if (offs + aclen > size)
> return (USBD_INVAL);
> ...
> if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
> aclen++;
> ...
> ibufend = ibuf + aclen;
>
> It looks more correct to fix aclen first, but does that still work ?
> e.g.:
> aclen = UGETW(acdp->wTotalLength);
> if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
> aclen++;
> if (offs + aclen > size)
> return (USBD_INVAL);
> ...
> ibufend = ibuf + aclen;
>
It might be 'better' logically speaking, but i know the (offs + aclen >
size) check wasnt triggered, as the debug printf 'found AC header' is
reached.
The one triggering the 'descriptor make no sense' error codepath is the
one checking (ibuf + dp->bLength > ibufend), and ibufend is computed
with aclen>
