[alsa-devel] [PATCH v3 2/3] ALSA: usb-audio: More strict sanity checks for clock parsers

[Takashi Iwai]

On Sat, 07 Apr 2018 02:55:23 +0200,
Ruslan Bilovol wrote:
> 
> On Fri, Apr 6, 2018 at 2:57 PM, Takashi Iwai <tiwai at suse.de> wrote:
> > On Fri, 06 Apr 2018 01:41:51 +0200,
> > Ruslan Bilovol wrote:
> >>
> >> Hi Takashi,
> >>
> >> On Thu, Apr 5, 2018 at 3:11 PM, Takashi Iwai <tiwai at suse.de> wrote:
> >> > The sanity checks introduced for malformed descriptors loosely check
> >> > the given descriptor size, although the size greater than the defined
> >> > description is invalid.  It was due to a concern of any funky firmware
> >> > in the actual products.  But this doesn't look hitting, and any sane
> >> > products must have the defined descriptors.
> >> >
> >> > So in this patch, we make the validators more strict, allowing only
> >> > with the defined descriptor sizes.
> >> >
> >> > Suggested-by: Ruslan Bilovol <ruslan.bilovol at gmail.com>
> >> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> >> > ---
> >> >  sound/usb/clock.c | 6 +++---
> >> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> >> > index 27c2275a2505..cbf68ab01836 100644
> >> > --- a/sound/usb/clock.c
> >> > +++ b/sound/usb/clock.c
> >> > @@ -52,7 +52,7 @@ static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
> >> >  static bool validate_clock_source_v2(void *p, int id)
> >> >  {
> >> >         struct uac_clock_source_descriptor *cs = p;
> >> > -       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
> >> > +       return cs->bLength == sizeof(*cs) && cs->bClockID == id;
> >> >  }
> >> >
> >> >  static bool validate_clock_source_v3(void *p, int id)
> >> > @@ -65,7 +65,7 @@ static bool validate_clock_selector_v2(void *p, int id)
> >> >  {
> >> >         struct uac_clock_selector_descriptor *cs = p;
> >> >         return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
> >> > -               cs->bLength >= 5 + cs->bNrInPins;
> >> > +               cs->bLength == 5 + cs->bNrInPins;
> >>
> >> This one still has an issue, here we should check it next way:
> >>                cs->bLength == 7 + cs->bNrInPins;
> >>
> >> This is because bLength is 7+bNrInPins as per UAC2 spec, not 5 :P
> >
> > Doh, I obviously overlooked the hidden members...
> > The fixed version is below.
> >
> >
> > Takashi
> >
> > -- 8< --
> > From: Takashi Iwai <tiwai at suse.de>
> > Subject: [PATCH v3] ALSA: usb-audio: More strict sanity checks for clock parsers
> >
> > The sanity checks introduced for malformed descriptors loosely check
> > the given descriptor size, although the size greater than the defined
> > description is invalid.  It was due to a concern of any funky firmware
> > in the actual products.  But this doesn't look hitting, and any sane
> > products must have the defined descriptors.
> >
> > So in this patch, we make the validators more strict, allowing only
> > with the defined descriptor sizes.  The value in clock selector
> > validator is corrected from 5 to 7 to count the two unlisted fields
> > after baCSourceID[].
> 
> Looks good!
> 
> Reviewed-by: Ruslan Bilovol <ruslan.bilovol at gmail.com>
> 
> I also tested this patch partially (validate_clock_source_v2() func only)
> and didn't get any issue in my case.

Great, I merged these three patches now .


thanks,

Takashi