[PATCH 0/3] 3 Patches for better UAC2 handling

[PATCH 0/3] 3 Patches for better UAC2 handling

[Daniel Mack]

Sorry, I've been distracted, but here are 3 patches that should be
included.

Thanks,
Daniel

Daniel Mack (2):
  ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6
  ALSA: snd-usb: operate on given mixer interface only

Nicolai Krakowiak (1):
  ALSA: snd-usb: avoid dividing by zero on invalid input

 sound/usb/endpoint.c |    2 +-
 sound/usb/mixer.c    |   25 +++++++++++++------------
 sound/usb/mixer.h    |    1 +
 3 files changed, 15 insertions(+), 13 deletions(-)

-- 
1.7.5.4

[PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

The Focusrite Scarlett 18i6 USB has them that way, which is probably a
bug. Anyway, the driver should simply ignore this fact.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
Cc: stable@kernel.org
---
 sound/usb/endpoint.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 1eea7e3..1867bbd 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
 			continue;
 		}
 		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
-		    ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
+		    ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
 			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n",
 				   dev->devnum, iface_no, altno, protocol, fmt->bLength);
 			continue;
-- 
1.7.5.4

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Takashi Iwai]

At Thu,  4 Aug 2011 15:56:26 +0200,
Daniel Mack wrote:
> 
> The Focusrite Scarlett 18i6 USB has them that way, which is probably a
> bug. Anyway, the driver should simply ignore this fact.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
> Cc: stable@kernel.org
> ---
>  sound/usb/endpoint.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 1eea7e3..1867bbd 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
>  			continue;
>  		}
>  		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
> -		    ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
> +		    ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
>  			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n",
>  				   dev->devnum, iface_no, altno, protocol, fmt->bLength);
>  			continue;

This patch isn't applicable to sound git tree.  Any missing patch?


thanks,

Takashi

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On Thu, Aug 4, 2011 at 4:07 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Thu,  4 Aug 2011 15:56:26 +0200,
> Daniel Mack wrote:
>>
>> The Focusrite Scarlett 18i6 USB has them that way, which is probably a
>> bug. Anyway, the driver should simply ignore this fact.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
>> Cc: stable@kernel.org
>> ---
>>  sound/usb/endpoint.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
>> index 1eea7e3..1867bbd 100644
>> --- a/sound/usb/endpoint.c
>> +++ b/sound/usb/endpoint.c
>> @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
>>                       continue;
>>               }
>>               if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
>> -                 ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
>> +                 ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
>>                       snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n",
>>                                  dev->devnum, iface_no, altno, protocol, fmt->bLength);
>>                       continue;
>
> This patch isn't applicable to sound git tree.  Any missing patch?

Eh, yes, sorry. Had it in my tree but did consider it important
enought to push right now. It's attached.

Thanks,
Daniel

[-- Attachment #2: 0001-ALSA-snd-usb-be-more-verbose-about-bogus-UAC_FORMAT_.patch --]
[-- Type: application/octet-stream, Size: 1037 bytes --]

From df652bed3931828471253d28dd7628b5a74145a3 Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque@gmail.com>
Date: Sat, 9 Jul 2011 16:02:42 +0200
Subject: [PATCH] ALSA: snd-usb: be more verbose about bogus UAC_FORMAT_TYPE
 descriptors

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 sound/usb/endpoint.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 7c0d21e..1eea7e3 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -353,8 +353,8 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
 		}
 		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
 		    ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
-			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc\n",
-				   dev->devnum, iface_no, altno);
+			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n",
+				   dev->devnum, iface_no, altno, protocol, fmt->bLength);
 			continue;
 		}
 
-- 
1.7.5.4


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Clemens Ladisch]

Takashi Iwai wrote:
> Daniel Mack wrote:
>> @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
>>  			continue;
>>  		}
>>  		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
>> -		    ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
>> +		    ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
>>  			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc (protocol %d, bLength %d)\n",
>>  				   dev->devnum, iface_no, altno, protocol, fmt->bLength);
>>  			continue;
> 
> This patch isn't applicable to sound git tree.  Any missing patch?

Obviously some temporary debugging patch to log protocol/bLength.

--8<---------------------------------------------------------------->8--
ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

The Focusrite Scarlett 18i6 USB has them that way, which is probably a
bug. Anyway, the driver should simply ignore this fact.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/usb/endpoint.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 1eea7e3..1867bbd 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
 			continue;
 		}
 		if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
-		    ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
+		    ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
 			snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc\n",
 				   dev->devnum, iface_no, altno);
 			continue;

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

On Thu, Aug 4, 2011 at 4:17 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
> Obviously some temporary debugging patch to log protocol/bLength.
>
> --8<---------------------------------------------------------------->8--
> ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6
>
> The Focusrite Scarlett 18i6 USB has them that way, which is probably a
> bug. Anyway, the driver should simply ignore this fact.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
> Cc: stable@kernel.org
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> ---
>  sound/usb/endpoint.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 1eea7e3..1867bbd 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
>                        continue;
>                }
>                if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
> -                   ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
> +                   ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
>                        snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc\n",
>                                   dev->devnum, iface_no, altno);
>                        continue;

Either that or apply the patch I've just sent out. Both is fine for me.

Daniel

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Takashi Iwai]

At Thu, 4 Aug 2011 16:17:09 +0200,
Daniel Mack wrote:
> 
> On Thu, Aug 4, 2011 at 4:17 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
> > Obviously some temporary debugging patch to log protocol/bLength.
> >
> > --8<---------------------------------------------------------------->8--
> > ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6
> >
> > The Focusrite Scarlett 18i6 USB has them that way, which is probably a
> > bug. Anyway, the driver should simply ignore this fact.
> >
> > Signed-off-by: Daniel Mack <zonque@gmail.com>
> > Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
> > Cc: stable@kernel.org
> > Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> > ---
> >  sound/usb/endpoint.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> > index 1eea7e3..1867bbd 100644
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -352,7 +352,7 @@ int snd_usb_parse_audio_endpoints(struct snd_usb_audio *chip, int iface_no)
> >                        continue;
> >                }
> >                if (((protocol == UAC_VERSION_1) && (fmt->bLength < 8)) ||
> > -                   ((protocol == UAC_VERSION_2) && (fmt->bLength != 6))) {
> > +                   ((protocol == UAC_VERSION_2) && (fmt->bLength < 6))) {
> >                        snd_printk(KERN_ERR "%d:%u:%d : invalid UAC_FORMAT_TYPE desc\n",
> >                                   dev->devnum, iface_no, altno);
> >                        continue;
> 
> Either that or apply the patch I've just sent out. Both is fine for me.

OK, I took the patch fixed by Clemens, since otherwise the patch isn't
applicable by itself alone to stable tree.
Other twos are applied now, too.

They'll be included in the next pull request to 3.1 (maybe after merge
window).


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

On Thu, Aug 4, 2011 at 4:27 PM, Takashi Iwai <tiwai@suse.de> wrote:
> They'll be included in the next pull request to 3.1 (maybe after merge
> window).

As they fix real bugs, don't you think it would be justified to go in
for 3.0? Also given that -rc1 isn't even released yet?


Thanks,
Daniel

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Takashi Iwai]

At Sat, 6 Aug 2011 09:26:11 +0200,
Daniel Mack wrote:
> 
> On Thu, Aug 4, 2011 at 4:27 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > They'll be included in the next pull request to 3.1 (maybe after merge
> > window).
> 
> As they fix real bugs, don't you think it would be justified to go in
> for 3.0? Also given that -rc1 isn't even released yet?

3.0 was finished.  The upcoming release is 3.1-rc1, but I don't want
to bother Linus too much during his vacation, unless a critical
regression (i.e. not "normal" bug) fix appears.

Anyway I'm going to send a pull request in the beginning of the next
week, so it'll fit into 3.1-rc2, at least.


Takashi

Re: [PATCH 1/3] ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6

[Daniel Mack]

On Aug 6, 2011 10:22 AM, "Takashi Iwai" <tiwai@suse.de> wrote:
>
> At Sat, 6 Aug 2011 09:26:11 +0200,
> Daniel Mack wrote:
> >
> > On Thu, Aug 4, 2011 at 4:27 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > > They'll be included in the next pull request to 3.1 (maybe after merge
> > > window).
> >
> > As they fix real bugs, don't you think it would be justified to go in
> > for 3.0? Also given that -rc1 isn't even released yet?
>
> 3.0 was finished.  The upcoming release is 3.1-rc1, but I don't want
> to bother Linus too much during his vacation, unless a critical
> regression (i.e. not "normal" bug) fix appears.

Tss, sorry. Early-morning confusion on my side of course.

Daniel

[PATCH 2/3] ALSA: snd-usb: avoid dividing by zero on invalid input

[Daniel Mack]

From: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>

Signed-off-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
Acked-by: Daniel Mack <zonque@gmail.com>
Cc: stable@kernel.org
---
 sound/usb/mixer.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index d33a5a9..5ab345f 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1191,6 +1191,11 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid, void
 
 	if (state->mixer->protocol == UAC_VERSION_1) {
 		csize = hdr->bControlSize;
+		if (!csize) {
+			snd_printdd(KERN_ERR "usbaudio: unit %u: "
+				    "invalid bControlSize == 0\n", unitid);
+			return -EINVAL;
+		}
 		channels = (hdr->bLength - 7) / csize - 1;
 		bmaControls = hdr->bmaControls;
 	} else {
-- 
1.7.5.4

[PATCH 3/3] ALSA: snd-usb: operate on given mixer interface only

[Daniel Mack]

When creating the mixers for an USB audio device, the current code looks
at the host interface stored in mixer->chip->ctrl_if. Change this and
rather keep a local pointer to the interface that was given when
snd_usb_create_mixer() was called.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-by: Nicolai Krakowiak <nicolai.krakowiak@gmail.com>
Reported-by: Lean-Yves LENHOF <jean-yves@lenhof.eu.org>
Cc: stable@kernel.org
---
 sound/usb/mixer.c |   20 ++++++++------------
 sound/usb/mixer.h |    1 +
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 5ab345f..45dd1fe 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1939,15 +1939,13 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 	struct mixer_build state;
 	int err;
 	const struct usbmix_ctl_map *map;
-	struct usb_host_interface *hostif;
 	void *p;
 
-	hostif = mixer->chip->ctrl_intf;
 	memset(&state, 0, sizeof(state));
 	state.chip = mixer->chip;
 	state.mixer = mixer;
-	state.buffer = hostif->extra;
-	state.buflen = hostif->extralen;
+	state.buffer = mixer->hostif->extra;
+	state.buflen = mixer->hostif->extralen;
 
 	/* check the mapping table */
 	for (map = usbmix_ctl_maps; map->id; map++) {
@@ -1960,7 +1958,8 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
 	}
 
 	p = NULL;
-	while ((p = snd_usb_find_csint_desc(hostif->extra, hostif->extralen, p, UAC_OUTPUT_TERMINAL)) != NULL) {
+	while ((p = snd_usb_find_csint_desc(mixer->hostif->extra, mixer->hostif->extralen,
+					    p, UAC_OUTPUT_TERMINAL)) != NULL) {
 		if (mixer->protocol == UAC_VERSION_1) {
 			struct uac1_output_terminal_descriptor *desc = p;
 
@@ -2167,17 +2166,15 @@ int snd_usb_mixer_activate(struct usb_mixer_interface *mixer)
 /* create the handler for the optional status interrupt endpoint */
 static int snd_usb_mixer_status_create(struct usb_mixer_interface *mixer)
 {
-	struct usb_host_interface *hostif;
 	struct usb_endpoint_descriptor *ep;
 	void *transfer_buffer;
 	int buffer_length;
 	unsigned int epnum;
 
-	hostif = mixer->chip->ctrl_intf;
 	/* we need one interrupt input endpoint */
-	if (get_iface_desc(hostif)->bNumEndpoints < 1)
+	if (get_iface_desc(mixer->hostif)->bNumEndpoints < 1)
 		return 0;
-	ep = get_endpoint(hostif, 0);
+	ep = get_endpoint(mixer->hostif, 0);
 	if (!usb_endpoint_dir_in(ep) || !usb_endpoint_xfer_int(ep))
 		return 0;
 
@@ -2207,7 +2204,6 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
 	};
 	struct usb_mixer_interface *mixer;
 	struct snd_info_entry *entry;
-	struct usb_host_interface *host_iface;
 	int err;
 
 	strcpy(chip->card->mixername, "USB Mixer");
@@ -2224,8 +2220,8 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
 		return -ENOMEM;
 	}
 
-	host_iface = &usb_ifnum_to_if(chip->dev, ctrlif)->altsetting[0];
-	switch (get_iface_desc(host_iface)->bInterfaceProtocol) {
+	mixer->hostif = &usb_ifnum_to_if(chip->dev, ctrlif)->altsetting[0];
+	switch (get_iface_desc(mixer->hostif)->bInterfaceProtocol) {
 	case UAC_VERSION_1:
 	default:
 		mixer->protocol = UAC_VERSION_1;
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index ae1a14d..81b2d8a 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -3,6 +3,7 @@
 
 struct usb_mixer_interface {
 	struct snd_usb_audio *chip;
+	struct usb_host_interface *hostif;
 	struct list_head list;
 	unsigned int ignore_ctl_error;
 	struct urb *urb;
-- 
1.7.5.4

Re: [PATCH 0/3] 3 Patches for better UAC2 handling

[Clemens Ladisch]

Daniel Mack wrote:
> Daniel Mack (2):
>   ALSA: snd-usb: Accept UAC2 FORMAT_TYPE descriptors with bLength > 6
>   ALSA: snd-usb: operate on given mixer interface only
> 
> Nicolai Krakowiak (1):
>   ALSA: snd-usb: avoid dividing by zero on invalid input

For all three:
Acked-by: Clemens Ladisch <clemens@ladisch.de>

> CC: <stable@kernel.org>

This must go _only_ into the patch description, because patches cannot
be submitted to the stable trees as long as they aren't yet in Linus'
tree.


Regards,
Clemens