[patch] Fix snd-usb-audio in 32-bit compat environemt

[patch] Fix snd-usb-audio in 32-bit compat environemt

[Juergen Kreileder]

Hi,

I'm getting oopses with snd-usb-audio in 32-bit compat environments:
control_compat.c:get_ctl_type() doesn't initialize 'info', so
'itemlist[uinfo->value.enumerated.item]' in
usbmixer.c:mixer_ctl_selector_info() might access random memory
(The 'if ((int)uinfo->value.enumerated.item >= cval->max)' doesn't fix
all problems because of the unsigned -> signed conversion.)

Here's a fix:

Signed-off-by: Juergen Kreileder <jk@blackdown.de>

--- linux-mm-vanilla/sound/core/control_compat.c	2006-02-18 17:00:17.000000000 +0100
+++ linux-mm/sound/core/control_compat.c	2006-02-18 19:17:45.000000000 +0100
@@ -167,7 +167,7 @@ static int get_ctl_type(struct snd_card 
 			int *countp)
 {
 	struct snd_kcontrol *kctl;
-	struct snd_ctl_elem_info info;
+	struct snd_ctl_elem_info *info;
 	int err;
 
 	down_read(&card->controls_rwsem);
@@ -176,13 +176,19 @@ static int get_ctl_type(struct snd_card 
 		up_read(&card->controls_rwsem);
 		return -ENXIO;
 	}
-	info.id = *id;
-	err = kctl->info(kctl, &info);
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (info == NULL) {
+		up_read(&card->controls_rwsem);
+		return -ENOMEM;
+	}
+	info->id = *id;
+	err = kctl->info(kctl, info);
 	up_read(&card->controls_rwsem);
 	if (err >= 0) {
-		err = info.type;
-		*countp = info.count;
+		err = info->type;
+		*countp = info->count;
 	}
+	kfree(info);
 	return err;
 }
 
=

Tested on ppc64.


        Juergen

-- 
Juergen Kreileder, Blackdown Java-Linux Team
http://blog.blackdown.de/


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

Re: [patch] Fix snd-usb-audio in 32-bit compat environemt

[Alexey Dobriyan]

On Sat, Feb 18, 2006 at 07:50:37PM +0100, Juergen Kreileder wrote:
> I'm getting oopses with snd-usb-audio in 32-bit compat environments:
> control_compat.c:get_ctl_type() doesn't initialize 'info', so
> 'itemlist[uinfo->value.enumerated.item]' in
> usbmixer.c:mixer_ctl_selector_info() might access random memory
> (The 'if ((int)uinfo->value.enumerated.item >= cval->max)' doesn't fix
> all problems because of the unsigned -> signed conversion.)

IMO, what you did is an overkill. Does this patch fixes your problem?

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

--- a/sound/core/control_compat.c
+++ b/sound/core/control_compat.c
@@ -176,6 +176,7 @@ static int get_ctl_type(struct snd_card
 		up_read(&card->controls_rwsem);
 		return -ENXIO;
 	}
+	memset(info, 0, sizeof(struct snd_ctl_elem_info));
 	info.id = *id;
 	err = kctl->info(kctl, &info);
 	up_read(&card->controls_rwsem);

> --- linux-mm-vanilla/sound/core/control_compat.c
> +++ linux-mm/sound/core/control_compat.c
> @@ -167,7 +167,7 @@ static int get_ctl_type(struct snd_card
>  			int *countp)
>  {
>  	struct snd_kcontrol *kctl;
> -	struct snd_ctl_elem_info info;
> +	struct snd_ctl_elem_info *info;
>  	int err;
>
>  	down_read(&card->controls_rwsem);
> @@ -176,13 +176,19 @@ static int get_ctl_type(struct snd_card
>  		up_read(&card->controls_rwsem);
>  		return -ENXIO;
>  	}
> -	info.id = *id;
> -	err = kctl->info(kctl, &info);
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (info == NULL) {
> +		up_read(&card->controls_rwsem);
> +		return -ENOMEM;
> +	}
> +	info->id = *id;
> +	err = kctl->info(kctl, info);
>  	up_read(&card->controls_rwsem);
>  	if (err >= 0) {
> -		err = info.type;
> -		*countp = info.count;
> +		err = info->type;
> +		*countp = info->count;
>  	}
> +	kfree(info);
>  	return err;
>  }



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

Re: [patch] Fix snd-usb-audio in 32-bit compat environemt

[Juergen Kreileder]

Alexey Dobriyan <adobriyan@gmail.com> writes:

> On Sat, Feb 18, 2006 at 07:50:37PM +0100, Juergen Kreileder wrote:
>> I'm getting oopses with snd-usb-audio in 32-bit compat
>> environments: control_compat.c:get_ctl_type() doesn't initialize
>> 'info', so
>> 'itemlist[uinfo->value.enumerated.item]' in
>> usbmixer.c:mixer_ctl_selector_info() might access random memory
>> (The 'if ((int)uinfo->value.enumerated.item >= cval->max)' doesn't fix
>> all problems because of the unsigned -> signed conversion.)
>
> IMO, what you did is an overkill.

The advantage of the longer fix is reduced stack usage.

> Does this patch fixes your problem?

Yes but you can have that even simpler:

--- linux-mm-vanilla/sound/core/control_compat.c	2006-02-18 17:00:17.000000000 +0100
+++ linux-mm/sound/core/control_compat.c	2006-02-19 19:41:51.000000000 +0100
@@ -167,7 +167,7 @@ static int get_ctl_type(struct snd_card 
 			int *countp)
 {
 	struct snd_kcontrol *kctl;
-	struct snd_ctl_elem_info info;
+	struct snd_ctl_elem_info info = {0};
 	int err;
 
 	down_read(&card->controls_rwsem);
=


        Juergen

-- 
Juergen Kreileder, Blackdown Java-Linux Team
http://blog.blackdown.de/

Re: [patch] Fix snd-usb-audio in 32-bit compat environemt

[Takashi Iwai]

At Sun, 19 Feb 2006 20:02:03 +0100,
Juergen Kreileder wrote:
> 
> Alexey Dobriyan <adobriyan@gmail.com> writes:
> 
> > On Sat, Feb 18, 2006 at 07:50:37PM +0100, Juergen Kreileder wrote:
> >> I'm getting oopses with snd-usb-audio in 32-bit compat
> >> environments: control_compat.c:get_ctl_type() doesn't initialize
> >> 'info', so
> >> 'itemlist[uinfo->value.enumerated.item]' in
> >> usbmixer.c:mixer_ctl_selector_info() might access random memory
> >> (The 'if ((int)uinfo->value.enumerated.item >= cval->max)' doesn't fix
> >> all problems because of the unsigned -> signed conversion.)
> >
> > IMO, what you did is an overkill.
> 
> The advantage of the longer fix is reduced stack usage.

But it also introduces a kmalloc overhead.
Let's fix the problem first with a simpler patch.

> > Does this patch fixes your problem?
> 
> Yes but you can have that even simpler:
> 
> --- linux-mm-vanilla/sound/core/control_compat.c	2006-02-18 17:00:17.000000000 +0100
> +++ linux-mm/sound/core/control_compat.c	2006-02-19 19:41:51.000000000 +0100
> @@ -167,7 +167,7 @@ static int get_ctl_type(struct snd_card 
>  			int *countp)
>  {
>  	struct snd_kcontrol *kctl;
> -	struct snd_ctl_elem_info info;
> +	struct snd_ctl_elem_info info = {0};
>  	int err;
>  
>  	down_read(&card->controls_rwsem);

I prefer Alexey's patch sicne it's clearer that the
zero-initialization is done just before the usage.

But = {0} might result in a better code because of compiler
optimization...


Thanks!

Takashi