[alsa-devel] [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers

[Takashi Iwai]

On Wed, 04 Apr 2018 01:15:05 +0200,
Ruslan Bilovol wrote:
> 
> On Tue, Apr 3, 2018 at 6:48 PM, Takashi Iwai <tiwai at suse.de> wrote:
> > The UAC3 clock parser codes lack of the sanity checks for malformed
> > descriptors like UAC2 parser does.  Without it, the driver may lead to
> > a potential crash.
> >
> > Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >  sound/usb/clock.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> > index c5f0cf532c0c..169fb3ac3715 100644
> > --- a/sound/usb/clock.c
> > +++ b/sound/usb/clock.c
> > @@ -58,7 +58,7 @@ static bool validate_clock_source_v2(void *p, int id)
> >  static bool validate_clock_source_v3(void *p, int id)
> >  {
> >         struct uac3_clock_source_descriptor *cs = p;
> > -       return cs->bClockID == id;
> > +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
> 
> I'm not sure why UAC2 checks are relaxed, but we can be more strict
> here since bLength of uac3_clock_source_descriptor is defined by standard
> and should be 12, so we can check for exact match in this place.
> 
> >  }
> >
> >  static bool validate_clock_selector_v2(void *p, int id)
> > @@ -71,7 +71,8 @@ static bool validate_clock_selector_v2(void *p, int id)
> >  static bool validate_clock_selector_v3(void *p, int id)
> >  {
> >         struct uac3_clock_selector_descriptor *cs = p;
> > -       return cs->bClockID == id;
> > +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
> > +               cs->bLength >= 5 + cs->bNrInPins;
> >  }
> 
> Same here, bLength is defined by spec, can be easily calculated and
> must be "11+bNrInPins"
> 
> >
> >  static bool validate_clock_multiplier_v2(void *p, int id)
> > @@ -83,7 +84,7 @@ static bool validate_clock_multiplier_v2(void *p, int id)
> >  static bool validate_clock_multiplier_v3(void *p, int id)
> >  {
> >         struct uac3_clock_multiplier_descriptor *cs = p;
> > -       return cs->bClockID == id;
> > +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
> 
> Also here, bLength should be 11 as per spec
> 
> By the way, we can make UAC2 bLength checks more strict as well,
> assuming there is no any hw bug we try to workaroud

OK, let's make the check more strict altogether.


thanks,

Takashi