2006-03-06 08:50:09

by Dave Jones

Subject: fix usbmixer double kfree.

snd_ctl_add() kfree's the kcontrol already if we fail there,
so this driver is currently doing a double kfree.

Coverity bug #959

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6/sound/usb/usbmixer.c~ 2006-03-06 03:40:20.000000000 -0500
+++ linux-2.6/sound/usb/usbmixer.c 2006-03-06 03:45:03.000000000 -0500
@@ -434,7 +434,6 @@ static int add_control_to_empty(struct m
kctl->id.index++;
if ((err = snd_ctl_add(state->chip->card, kctl)) < 0) {
snd_printd(KERN_ERR "cannot add control (err = %d)\n", err);
- snd_ctl_free_one(kctl);
return err;
}
cval->elem_id = &kctl->id;

--
http://www.codemonkey.org.uk

2006-03-06 13:43:12

by Takashi Iwai

Subject: Re: fix usbmixer double kfree.

At Mon, 6 Mar 2006 03:49:51 -0500,
Dave Jones wrote:
>
> snd_ctl_add() kfree's the kcontrol already if we fail there,
> so this driver is currently doing a double kfree.
>
> Coverity bug #959
>
> Signed-off-by: Dave Jones <[email protected]>
>
> --- linux-2.6/sound/usb/usbmixer.c~ 2006-03-06 03:40:20.000000000 -0500
> +++ linux-2.6/sound/usb/usbmixer.c 2006-03-06 03:45:03.000000000 -0500
> @@ -434,7 +434,6 @@ static int add_control_to_empty(struct m
> kctl->id.index++;
> if ((err = snd_ctl_add(state->chip->card, kctl)) < 0) {
> snd_printd(KERN_ERR "cannot add control (err = %d)\n", err);
> - snd_ctl_free_one(kctl);
> return err;
> }
> cval->elem_id = &kctl->id;
>


Thanks, applied to ALSA tree now, too.

Signed-off-by: Takashi Iwai <[email protected]>


Takashi

2006-03-07 00:43:16

by Andrew Morton

Subject: Re: fix usbmixer double kfree.

Dave Jones <[email protected]> wrote:
>
> snd_ctl_add() kfree's the kcontrol already if we fail there,
> so this driver is currently doing a double kfree.

Well sometimes it does. If we hit one of those snd_assert() abominations,
snd_ctl_add() will return error without freeing the kcontrol.

Still, a leak is better than a double-free.

> --- linux-2.6/sound/usb/usbmixer.c~ 2006-03-06 03:40:20.000000000 -0500
> +++ linux-2.6/sound/usb/usbmixer.c 2006-03-06 03:45:03.000000000 -0500
> @@ -434,7 +434,6 @@ static int add_control_to_empty(struct m
> kctl->id.index++;
> if ((err = snd_ctl_add(state->chip->card, kctl)) < 0) {
> snd_printd(KERN_ERR "cannot add control (err = %d)\n", err);
> - snd_ctl_free_one(kctl);
> return err;
> }
> cval->elem_id = &kctl->id;

2006-03-07 11:13:48

by Takashi Iwai

Subject: Re: fix usbmixer double kfree.

At Mon, 6 Mar 2006 16:41:11 -0800,
Andrew Morton wrote:
>
> Dave Jones <[email protected]> wrote:
> >
> > snd_ctl_add() kfree's the kcontrol already if we fail there,
> > so this driver is currently doing a double kfree.
>
> Well sometimes it does. If we hit one of those snd_assert() abominations,
> snd_ctl_add() will return error without freeing the kcontrol.

Indeed. I'll fix them later.

> Still, a leak is better than a double-free.

Yep :)


thanks,

Takashi