All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers
@ 2018-04-03 15:48 Takashi Iwai
  2018-04-03 15:48 ` [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Takashi Iwai
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Takashi Iwai @ 2018-04-03 15:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Ruslan Bilovol

There are lots of open-coded functions to find a clock source,
selector and multiplier.  Now there are both v2 and v3, so six
variants.

This patch refactors the code to use a common helper for the main
loop, and define each validator function for each target.
There is no functional change.

Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
 1 file changed, 53 insertions(+), 74 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index ab39ccb974c6..c5f0cf532c0c 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -35,105 +35,84 @@
 #include "clock.h"
 #include "quirks.h"
 
-static struct uac_clock_source_descriptor *
-	snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
-				  int clock_id)
+static void *find_descriptor(struct usb_host_interface *iface, int id,
+			     bool (*validator)(void *, int), u8 type)
 {
-	struct uac_clock_source_descriptor *cs = NULL;
+	void *cs = NULL;
 
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_SOURCE))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
+	while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
+					     cs, type))) {
+		if (validator(cs, id))
 			return cs;
 	}
 
 	return NULL;
 }
 
-static struct uac3_clock_source_descriptor *
-	snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
-				  int clock_id)
+static bool validate_clock_source_v2(void *p, int id)
 {
-	struct uac3_clock_source_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_SOURCE))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac_clock_source_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
 }
 
-static struct uac_clock_selector_descriptor *
-	snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
-				    int clock_id)
+static bool validate_clock_source_v3(void *p, int id)
 {
-	struct uac_clock_selector_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_SELECTOR))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) {
-			if (cs->bLength < 5 + cs->bNrInPins)
-				return NULL;
-			return cs;
-		}
-	}
-
-	return NULL;
+	struct uac3_clock_source_descriptor *cs = p;
+	return cs->bClockID == id;
 }
 
-static struct uac3_clock_selector_descriptor *
-	snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
-				    int clock_id)
+static bool validate_clock_selector_v2(void *p, int id)
 {
-	struct uac3_clock_selector_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_SELECTOR))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac_clock_selector_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
+		cs->bLength >= 5 + cs->bNrInPins;
 }
 
-static struct uac_clock_multiplier_descriptor *
-	snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
-				      int clock_id)
+static bool validate_clock_selector_v3(void *p, int id)
 {
-	struct uac_clock_multiplier_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_MULTIPLIER))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac3_clock_selector_descriptor *cs = p;
+	return cs->bClockID == id;
 }
 
-static struct uac3_clock_multiplier_descriptor *
-	snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
-				      int clock_id)
+static bool validate_clock_multiplier_v2(void *p, int id)
 {
-	struct uac3_clock_multiplier_descriptor *cs = NULL;
+	struct uac_clock_multiplier_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
+}
 
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_MULTIPLIER))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
+static bool validate_clock_multiplier_v3(void *p, int id)
+{
+	struct uac3_clock_multiplier_descriptor *cs = p;
+	return cs->bClockID == id;
+}
 
-	return NULL;
+#define DEFINE_FIND_HELPER(name, obj, validator, type)		\
+static obj *name(struct usb_host_interface *iface, int id)	\
+{								\
+	return find_descriptor(iface, id, validator, type);	\
 }
 
+DEFINE_FIND_HELPER(snd_usb_find_clock_source,
+		   struct uac_clock_source_descriptor,
+		   validate_clock_source_v2, UAC2_CLOCK_SOURCE);
+DEFINE_FIND_HELPER(snd_usb_find_clock_source_v3,
+		   struct uac3_clock_source_descriptor,
+		   validate_clock_source_v3, UAC3_CLOCK_SOURCE);
+
+DEFINE_FIND_HELPER(snd_usb_find_clock_selector,
+		   struct uac_clock_selector_descriptor,
+		   validate_clock_selector_v2, UAC2_CLOCK_SELECTOR);
+DEFINE_FIND_HELPER(snd_usb_find_clock_selector_v3,
+		   struct uac3_clock_selector_descriptor,
+		   validate_clock_selector_v3, UAC3_CLOCK_SELECTOR);
+
+DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier,
+		   struct uac_clock_multiplier_descriptor,
+		   validate_clock_multiplier_v2, UAC2_CLOCK_MULTIPLIER);
+DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier_v3,
+		   struct uac3_clock_multiplier_descriptor,
+		   validate_clock_multiplier_v3, UAC3_CLOCK_MULTIPLIER);
+
 static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
 {
 	unsigned char buf;
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers
  2018-04-03 15:48 [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers Takashi Iwai
@ 2018-04-03 15:48 ` Takashi Iwai
  2018-04-03 17:23   ` Andrew Chant
  2018-04-03 23:15   ` Ruslan Bilovol
  2018-04-03 17:18 ` [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers Andrew Chant
       [not found] ` <CANVWZtGDxQ2nRjyy15BPgrf0i5FuRxrXkRB1ae72GDoVDExF1Q@mail.gmail.com>
  2 siblings, 2 replies; 8+ messages in thread
From: Takashi Iwai @ 2018-04-03 15:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Ruslan Bilovol

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@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;
 }
 
 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;
 }
 
 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;
 }
 
 #define DEFINE_FIND_HELPER(name, obj, validator, type)		\
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers
  2018-04-03 15:48 [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers Takashi Iwai
  2018-04-03 15:48 ` [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Takashi Iwai
@ 2018-04-03 17:18 ` Andrew Chant
       [not found] ` <CANVWZtGDxQ2nRjyy15BPgrf0i5FuRxrXkRB1ae72GDoVDExF1Q@mail.gmail.com>
  2 siblings, 0 replies; 8+ messages in thread
From: Andrew Chant @ 2018-04-03 17:18 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Ruslan Bilovol

On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai <tiwai@suse.de> wrote:
>
> There are lots of open-coded functions to find a clock source,
> selector and multiplier.  Now there are both v2 and v3, so six
> variants.
>
> This patch refactors the code to use a common helper for the main
> loop, and define each validator function for each target.
> There is no functional change.
>
> Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
>  1 file changed, 53 insertions(+), 74 deletions(-)
>
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index ab39ccb974c6..c5f0cf532c0c 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -35,105 +35,84 @@
>  #include "clock.h"
>  #include "quirks.h"
>
> -static struct uac_clock_source_descriptor *
> -       snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
> -                                 int clock_id)
> +static void *find_descriptor(struct usb_host_interface *iface, int id,
> +                            bool (*validator)(void *, int), u8 type)

this comment isn't very important, and might be wrong:
find_descriptor is a very generic name, it might make grepping a
little confusing.
Could you use find_clock_descriptor? find_uac_clock_descriptor?

>  {
> -       struct uac_clock_source_descriptor *cs = NULL;
> +       void *cs = NULL;
>
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_SOURCE))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
> +       while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
> +                                            cs, type))) {
> +               if (validator(cs, id))
>                         return cs;
>         }
>
>         return NULL;
>  }
>
> -static struct uac3_clock_source_descriptor *
> -       snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
> -                                 int clock_id)
> +static bool validate_clock_source_v2(void *p, int id)
>  {
> -       struct uac3_clock_source_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_SOURCE))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac_clock_source_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
>  }
>
> -static struct uac_clock_selector_descriptor *
> -       snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
> -                                   int clock_id)
> +static bool validate_clock_source_v3(void *p, int id)
>  {
> -       struct uac_clock_selector_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_SELECTOR))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) {
> -                       if (cs->bLength < 5 + cs->bNrInPins)
> -                               return NULL;
> -                       return cs;
> -               }
> -       }
> -
> -       return NULL;
> +       struct uac3_clock_source_descriptor *cs = p;
> +       return cs->bClockID == id;

uac3_clock_source_descriptor has a fixed size (12).
Consider a sizeof() check here to be consistent?

>  }
>
> -static struct uac3_clock_selector_descriptor *
> -       snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
> -                                   int clock_id)
> +static bool validate_clock_selector_v2(void *p, int id)
>  {
> -       struct uac3_clock_selector_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_SELECTOR))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac_clock_selector_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
> +               cs->bLength >= 5 + cs->bNrInPins;
>  }
>
> -static struct uac_clock_multiplier_descriptor *
> -       snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
> -                                     int clock_id)
> +static bool validate_clock_selector_v3(void *p, int id)
>  {
> -       struct uac_clock_multiplier_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_MULTIPLIER))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac3_clock_selector_descriptor *cs = p;
> +       return cs->bClockID == id;
clock cs->bLength should be 11+ bytes for a v3 clock selector descriptor.
consider an >= size check. (I don't know how sizeof works with
[]-terminated structs)

>  }
>
> -static struct uac3_clock_multiplier_descriptor *
> -       snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
> -                                     int clock_id)
> +static bool validate_clock_multiplier_v2(void *p, int id)
>  {
> -       struct uac3_clock_multiplier_descriptor *cs = NULL;
> +       struct uac_clock_multiplier_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
> +}
>
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_MULTIPLIER))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> +static bool validate_clock_multiplier_v3(void *p, int id)
> +{
> +       struct uac3_clock_multiplier_descriptor *cs = p;
> +       return cs->bClockID == id;
uac3_clock_multiplier_descriptor has fixed size (11)

> +}
>
> -       return NULL;
> +#define DEFINE_FIND_HELPER(name, obj, validator, type)         \
> +static obj *name(struct usb_host_interface *iface, int id)     \
> +{                                                              \
> +       return find_descriptor(iface, id, validator, type);     \
>  }
>
> +DEFINE_FIND_HELPER(snd_usb_find_clock_source,
> +                  struct uac_clock_source_descriptor,
> +                  validate_clock_source_v2, UAC2_CLOCK_SOURCE);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_source_v3,
> +                  struct uac3_clock_source_descriptor,
> +                  validate_clock_source_v3, UAC3_CLOCK_SOURCE);
> +
> +DEFINE_FIND_HELPER(snd_usb_find_clock_selector,
> +                  struct uac_clock_selector_descriptor,
> +                  validate_clock_selector_v2, UAC2_CLOCK_SELECTOR);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_selector_v3,
> +                  struct uac3_clock_selector_descriptor,
> +                  validate_clock_selector_v3, UAC3_CLOCK_SELECTOR);
> +
> +DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier,
> +                  struct uac_clock_multiplier_descriptor,
> +                  validate_clock_multiplier_v2, UAC2_CLOCK_MULTIPLIER);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier_v3,
> +                  struct uac3_clock_multiplier_descriptor,
> +                  validate_clock_multiplier_v3, UAC3_CLOCK_MULTIPLIER);
> +
>  static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
>  {
>         unsigned char buf;
> --
> 2.16.2
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers
  2018-04-03 15:48 ` [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Takashi Iwai
@ 2018-04-03 17:23   ` Andrew Chant
  2018-04-03 23:15   ` Ruslan Bilovol
  1 sibling, 0 replies; 8+ messages in thread
From: Andrew Chant @ 2018-04-03 17:23 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Ruslan Bilovol

On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai <tiwai@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@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;
>  }
>
>  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;
>  }
>
>  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;
>  }
>
>  #define DEFINE_FIND_HELPER(name, obj, validator, type)         \
> --
> 2.16.2
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

These address all the comments I had on patch 1. sorry for the noise.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers
       [not found] ` <CANVWZtGDxQ2nRjyy15BPgrf0i5FuRxrXkRB1ae72GDoVDExF1Q@mail.gmail.com>
@ 2018-04-03 18:02   ` Takashi Iwai
  2018-04-03 22:31     ` Ruslan Bilovol
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2018-04-03 18:02 UTC (permalink / raw)
  To: Andrew Chant; +Cc: alsa-devel, Ruslan Bilovol

On Tue, 03 Apr 2018 19:13:51 +0200,
Andrew Chant wrote:
> 
> On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >
> > There are lots of open-coded functions to find a clock source,
> > selector and multiplier.  Now there are both v2 and v3, so six
> > variants.
> >
> > This patch refactors the code to use a common helper for the main
> > loop, and define each validator function for each target.
> > There is no functional change.
> >
> > Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
> >  1 file changed, 53 insertions(+), 74 deletions(-)
> >
> > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> > index ab39ccb974c6..c5f0cf532c0c 100644
> > --- a/sound/usb/clock.c
> > +++ b/sound/usb/clock.c
> > @@ -35,105 +35,84 @@
> >  #include "clock.h"
> >  #include "quirks.h"
> >
> > -static struct uac_clock_source_descriptor *
> > -       snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
> > -                                 int clock_id)
> > +static void *find_descriptor(struct usb_host_interface *iface, int id,
> > +                            bool (*validator)(void *, int), u8 type)
> 
> this comment isn't very important, and might be wrong:
> find_descriptor is a very generic name, it might make grepping a
> little confusing.
> Could you use find_clock_descriptor? find_uac_clock_descriptor?

Makes sense.  Now I renamed it with a bit shorter one,
find_uac_clock_desc().  The revised patch is below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2 1/2] ALSA: usb-audio: Refactor clock finder helpers

There are lots of open-coded functions to find a clock source,
selector and multiplier.  Now there are both v2 and v3, so six
variants.

This patch refactors the code to use a common helper for the main
loop, and define each validator function for each target.
There is no functional change.

Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: rename find_descriptor() with find_uac_clock_desc()

 sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
 1 file changed, 53 insertions(+), 74 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index ab39ccb974c6..27c2275a2505 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -35,105 +35,84 @@
 #include "clock.h"
 #include "quirks.h"
 
-static struct uac_clock_source_descriptor *
-	snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
-				  int clock_id)
+static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
+				 bool (*validator)(void *, int), u8 type)
 {
-	struct uac_clock_source_descriptor *cs = NULL;
+	void *cs = NULL;
 
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_SOURCE))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
+	while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
+					     cs, type))) {
+		if (validator(cs, id))
 			return cs;
 	}
 
 	return NULL;
 }
 
-static struct uac3_clock_source_descriptor *
-	snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
-				  int clock_id)
+static bool validate_clock_source_v2(void *p, int id)
 {
-	struct uac3_clock_source_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_SOURCE))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac_clock_source_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
 }
 
-static struct uac_clock_selector_descriptor *
-	snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
-				    int clock_id)
+static bool validate_clock_source_v3(void *p, int id)
 {
-	struct uac_clock_selector_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_SELECTOR))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) {
-			if (cs->bLength < 5 + cs->bNrInPins)
-				return NULL;
-			return cs;
-		}
-	}
-
-	return NULL;
+	struct uac3_clock_source_descriptor *cs = p;
+	return cs->bClockID == id;
 }
 
-static struct uac3_clock_selector_descriptor *
-	snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
-				    int clock_id)
+static bool validate_clock_selector_v2(void *p, int id)
 {
-	struct uac3_clock_selector_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_SELECTOR))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac_clock_selector_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
+		cs->bLength >= 5 + cs->bNrInPins;
 }
 
-static struct uac_clock_multiplier_descriptor *
-	snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
-				      int clock_id)
+static bool validate_clock_selector_v3(void *p, int id)
 {
-	struct uac_clock_multiplier_descriptor *cs = NULL;
-
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC2_CLOCK_MULTIPLIER))) {
-		if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
-			return cs;
-	}
-
-	return NULL;
+	struct uac3_clock_selector_descriptor *cs = p;
+	return cs->bClockID == id;
 }
 
-static struct uac3_clock_multiplier_descriptor *
-	snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
-				      int clock_id)
+static bool validate_clock_multiplier_v2(void *p, int id)
 {
-	struct uac3_clock_multiplier_descriptor *cs = NULL;
+	struct uac_clock_multiplier_descriptor *cs = p;
+	return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
+}
 
-	while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
-					     ctrl_iface->extralen,
-					     cs, UAC3_CLOCK_MULTIPLIER))) {
-		if (cs->bClockID == clock_id)
-			return cs;
-	}
+static bool validate_clock_multiplier_v3(void *p, int id)
+{
+	struct uac3_clock_multiplier_descriptor *cs = p;
+	return cs->bClockID == id;
+}
 
-	return NULL;
+#define DEFINE_FIND_HELPER(name, obj, validator, type)		\
+static obj *name(struct usb_host_interface *iface, int id)	\
+{								\
+	return find_uac_clock_desc(iface, id, validator, type);	\
 }
 
+DEFINE_FIND_HELPER(snd_usb_find_clock_source,
+		   struct uac_clock_source_descriptor,
+		   validate_clock_source_v2, UAC2_CLOCK_SOURCE);
+DEFINE_FIND_HELPER(snd_usb_find_clock_source_v3,
+		   struct uac3_clock_source_descriptor,
+		   validate_clock_source_v3, UAC3_CLOCK_SOURCE);
+
+DEFINE_FIND_HELPER(snd_usb_find_clock_selector,
+		   struct uac_clock_selector_descriptor,
+		   validate_clock_selector_v2, UAC2_CLOCK_SELECTOR);
+DEFINE_FIND_HELPER(snd_usb_find_clock_selector_v3,
+		   struct uac3_clock_selector_descriptor,
+		   validate_clock_selector_v3, UAC3_CLOCK_SELECTOR);
+
+DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier,
+		   struct uac_clock_multiplier_descriptor,
+		   validate_clock_multiplier_v2, UAC2_CLOCK_MULTIPLIER);
+DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier_v3,
+		   struct uac3_clock_multiplier_descriptor,
+		   validate_clock_multiplier_v3, UAC3_CLOCK_MULTIPLIER);
+
 static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
 {
 	unsigned char buf;
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers
  2018-04-03 18:02   ` Takashi Iwai
@ 2018-04-03 22:31     ` Ruslan Bilovol
  0 siblings, 0 replies; 8+ messages in thread
From: Ruslan Bilovol @ 2018-04-03 22:31 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Andrew Chant

On Tue, Apr 3, 2018 at 9:02 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 03 Apr 2018 19:13:51 +0200,
> Andrew Chant wrote:
>>
>> On Tue, Apr 3, 2018 at 8:48 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >
>> > There are lots of open-coded functions to find a clock source,
>> > selector and multiplier.  Now there are both v2 and v3, so six
>> > variants.
>> >
>> > This patch refactors the code to use a common helper for the main
>> > loop, and define each validator function for each target.
>> > There is no functional change.
>> >
>> > Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
>> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
>> > ---
>> >  sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
>> >  1 file changed, 53 insertions(+), 74 deletions(-)
>> >
>> > diff --git a/sound/usb/clock.c b/sound/usb/clock.c
>> > index ab39ccb974c6..c5f0cf532c0c 100644
>> > --- a/sound/usb/clock.c
>> > +++ b/sound/usb/clock.c
>> > @@ -35,105 +35,84 @@
>> >  #include "clock.h"
>> >  #include "quirks.h"
>> >
>> > -static struct uac_clock_source_descriptor *
>> > -       snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
>> > -                                 int clock_id)
>> > +static void *find_descriptor(struct usb_host_interface *iface, int id,
>> > +                            bool (*validator)(void *, int), u8 type)
>>
>> this comment isn't very important, and might be wrong:
>> find_descriptor is a very generic name, it might make grepping a
>> little confusing.
>> Could you use find_clock_descriptor? find_uac_clock_descriptor?
>
> Makes sense.  Now I renamed it with a bit shorter one,
> find_uac_clock_desc().  The revised patch is below.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH v2 1/2] ALSA: usb-audio: Refactor clock finder helpers
>
> There are lots of open-coded functions to find a clock source,
> selector and multiplier.  Now there are both v2 and v3, so six
> variants.
>
> This patch refactors the code to use a common helper for the main
> loop, and define each validator function for each target.
> There is no functional change.
>
> Fixes: 9a2fe9b801f5 ("ALSA: usb: initial USB Audio Device Class 3.0 support")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Good improvement.

Reviewed-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>

> ---
> v1->v2: rename find_descriptor() with find_uac_clock_desc()
>
>  sound/usb/clock.c | 127 +++++++++++++++++++++++-------------------------------
>  1 file changed, 53 insertions(+), 74 deletions(-)
>
> diff --git a/sound/usb/clock.c b/sound/usb/clock.c
> index ab39ccb974c6..27c2275a2505 100644
> --- a/sound/usb/clock.c
> +++ b/sound/usb/clock.c
> @@ -35,105 +35,84 @@
>  #include "clock.h"
>  #include "quirks.h"
>
> -static struct uac_clock_source_descriptor *
> -       snd_usb_find_clock_source(struct usb_host_interface *ctrl_iface,
> -                                 int clock_id)
> +static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
> +                                bool (*validator)(void *, int), u8 type)
>  {
> -       struct uac_clock_source_descriptor *cs = NULL;
> +       void *cs = NULL;
>
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_SOURCE))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
> +       while ((cs = snd_usb_find_csint_desc(iface->extra, iface->extralen,
> +                                            cs, type))) {
> +               if (validator(cs, id))
>                         return cs;
>         }
>
>         return NULL;
>  }
>
> -static struct uac3_clock_source_descriptor *
> -       snd_usb_find_clock_source_v3(struct usb_host_interface *ctrl_iface,
> -                                 int clock_id)
> +static bool validate_clock_source_v2(void *p, int id)
>  {
> -       struct uac3_clock_source_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_SOURCE))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac_clock_source_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
>  }
>
> -static struct uac_clock_selector_descriptor *
> -       snd_usb_find_clock_selector(struct usb_host_interface *ctrl_iface,
> -                                   int clock_id)
> +static bool validate_clock_source_v3(void *p, int id)
>  {
> -       struct uac_clock_selector_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_SELECTOR))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id) {
> -                       if (cs->bLength < 5 + cs->bNrInPins)
> -                               return NULL;
> -                       return cs;
> -               }
> -       }
> -
> -       return NULL;
> +       struct uac3_clock_source_descriptor *cs = p;
> +       return cs->bClockID == id;
>  }
>
> -static struct uac3_clock_selector_descriptor *
> -       snd_usb_find_clock_selector_v3(struct usb_host_interface *ctrl_iface,
> -                                   int clock_id)
> +static bool validate_clock_selector_v2(void *p, int id)
>  {
> -       struct uac3_clock_selector_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_SELECTOR))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac_clock_selector_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id &&
> +               cs->bLength >= 5 + cs->bNrInPins;
>  }
>
> -static struct uac_clock_multiplier_descriptor *
> -       snd_usb_find_clock_multiplier(struct usb_host_interface *ctrl_iface,
> -                                     int clock_id)
> +static bool validate_clock_selector_v3(void *p, int id)
>  {
> -       struct uac_clock_multiplier_descriptor *cs = NULL;
> -
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC2_CLOCK_MULTIPLIER))) {
> -               if (cs->bLength >= sizeof(*cs) && cs->bClockID == clock_id)
> -                       return cs;
> -       }
> -
> -       return NULL;
> +       struct uac3_clock_selector_descriptor *cs = p;
> +       return cs->bClockID == id;
>  }
>
> -static struct uac3_clock_multiplier_descriptor *
> -       snd_usb_find_clock_multiplier_v3(struct usb_host_interface *ctrl_iface,
> -                                     int clock_id)
> +static bool validate_clock_multiplier_v2(void *p, int id)
>  {
> -       struct uac3_clock_multiplier_descriptor *cs = NULL;
> +       struct uac_clock_multiplier_descriptor *cs = p;
> +       return cs->bLength >= sizeof(*cs) && cs->bClockID == id;
> +}
>
> -       while ((cs = snd_usb_find_csint_desc(ctrl_iface->extra,
> -                                            ctrl_iface->extralen,
> -                                            cs, UAC3_CLOCK_MULTIPLIER))) {
> -               if (cs->bClockID == clock_id)
> -                       return cs;
> -       }
> +static bool validate_clock_multiplier_v3(void *p, int id)
> +{
> +       struct uac3_clock_multiplier_descriptor *cs = p;
> +       return cs->bClockID == id;
> +}
>
> -       return NULL;
> +#define DEFINE_FIND_HELPER(name, obj, validator, type)         \
> +static obj *name(struct usb_host_interface *iface, int id)     \
> +{                                                              \
> +       return find_uac_clock_desc(iface, id, validator, type); \
>  }
>
> +DEFINE_FIND_HELPER(snd_usb_find_clock_source,
> +                  struct uac_clock_source_descriptor,
> +                  validate_clock_source_v2, UAC2_CLOCK_SOURCE);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_source_v3,
> +                  struct uac3_clock_source_descriptor,
> +                  validate_clock_source_v3, UAC3_CLOCK_SOURCE);
> +
> +DEFINE_FIND_HELPER(snd_usb_find_clock_selector,
> +                  struct uac_clock_selector_descriptor,
> +                  validate_clock_selector_v2, UAC2_CLOCK_SELECTOR);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_selector_v3,
> +                  struct uac3_clock_selector_descriptor,
> +                  validate_clock_selector_v3, UAC3_CLOCK_SELECTOR);
> +
> +DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier,
> +                  struct uac_clock_multiplier_descriptor,
> +                  validate_clock_multiplier_v2, UAC2_CLOCK_MULTIPLIER);
> +DEFINE_FIND_HELPER(snd_usb_find_clock_multiplier_v3,
> +                  struct uac3_clock_multiplier_descriptor,
> +                  validate_clock_multiplier_v3, UAC3_CLOCK_MULTIPLIER);
> +
>  static int uac_clock_selector_get_val(struct snd_usb_audio *chip, int selector_id)
>  {
>         unsigned char buf;
> --
> 2.16.2
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers
  2018-04-03 15:48 ` [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Takashi Iwai
  2018-04-03 17:23   ` Andrew Chant
@ 2018-04-03 23:15   ` Ruslan Bilovol
  2018-04-04  5:34     ` Takashi Iwai
  1 sibling, 1 reply; 8+ messages in thread
From: Ruslan Bilovol @ 2018-04-03 23:15 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Tue, Apr 3, 2018 at 6:48 PM, Takashi Iwai <tiwai@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@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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers
  2018-04-03 23:15   ` Ruslan Bilovol
@ 2018-04-04  5:34     ` Takashi Iwai
  0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2018-04-04  5:34 UTC (permalink / raw)
  To: Ruslan Bilovol; +Cc: alsa-devel

On Wed, 04 Apr 2018 01:15:05 +0200,
Ruslan Bilovol wrote:
> 
> On Tue, Apr 3, 2018 at 6:48 PM, Takashi Iwai <tiwai@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@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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-04-04  5:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 15:48 [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers Takashi Iwai
2018-04-03 15:48 ` [PATCH 2/2] ALSA: usb-audio: Add sanity checks in v3 clock parsers Takashi Iwai
2018-04-03 17:23   ` Andrew Chant
2018-04-03 23:15   ` Ruslan Bilovol
2018-04-04  5:34     ` Takashi Iwai
2018-04-03 17:18 ` [PATCH 1/2] ALSA: usb-audio: Refactor clock finder helpers Andrew Chant
     [not found] ` <CANVWZtGDxQ2nRjyy15BPgrf0i5FuRxrXkRB1ae72GDoVDExF1Q@mail.gmail.com>
2018-04-03 18:02   ` Takashi Iwai
2018-04-03 22:31     ` Ruslan Bilovol

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.