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
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
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;
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