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

[Ruslan Bilovol]

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

Thanks,
Ruslan