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

[Takashi Iwai]

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[].

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..30cfd5b1bdfb 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 == 7 + cs->bNrInPins;
 }
 
 static bool validate_clock_selector_v3(void *p, int id)
@@ -77,7 +77,7 @@ static bool validate_clock_selector_v3(void *p, int id)
 static bool validate_clock_multiplier_v2(void *p, int id)
 {
 	struct uac_clock_multiplier_descriptor *cs = p;
-	return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
+	return cs->bLength == sizeof(*cs) && cs->bClockID == id;
 }
 
 static bool validate_clock_multiplier_v3(void *p, int id)
-- 
2.16.3