Re: [PATCH 02/16] ALSA: line6: Fix memory leak at probe error path

[Takashi Iwai <tiwai@suse.de>]

At Sun, 25 Jan 2015 02:06:19 -0600,
Chris Rorvick wrote:
> 
> On Fri, Jan 23, 2015 at 11:13 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > --- a/sound/usb/line6/driver.c
> > +++ b/sound/usb/line6/driver.c
> > @@ -507,10 +507,32 @@ int line6_probe(struct usb_interface *interface,
> >         int interface_number;
> >         int ret;
> >
> > +       ret = snd_card_new(line6->ifcdev,
> > +                          SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
> > +                          THIS_MODULE, 0, &card);
> 
> The `ifcdev' member has not been initialized yet.

Good catch.

> You need something
> like the following squashed in:
> 
> >8------------------------------------------------------8<
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 2eed6fb..25d6b0f 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -507,6 +507,11 @@ int line6_probe(struct usb_interface *interface,
>         int interface_number;
>         int ret;
> 
> +       /* store basic data: */
> +       line6->properties = properties;
> +       line6->usbdev = usbdev;
> +       line6->ifcdev = &interface->dev;
> +
>         ret = snd_card_new(line6->ifcdev,
>                            SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
>                            THIS_MODULE, 0, &card);
> @@ -548,11 +553,6 @@ int line6_probe(struct usb_interface *interface,
>                 goto error;
>         }
> 
> -       /* store basic data: */
> -       line6->properties = properties;
> -       line6->usbdev = usbdev;
> -       line6->ifcdev = &interface->dev;
> -
>         line6_get_interval(line6);
> 
>         if (properties->capabilities & LINE6_CAP_CONTROL) {

Right, but there will be another round of cleanup patches and the
allocation of line6 will be dropped there, so I took an easier fixup,
just use &interface->dev to the argument.

The revised patch is found below.  The test/line6 branch was updated
with this (and more fixes I'll send shortly later).


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: line6: Fix memory leak at probe error path

Fix memory leak at probe error path by rearranging the call order in
line6_destruct() so that the common destructor is always called.
Also this simplifies the error path to a single goto label.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/line6/driver.c | 59 ++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index b783c0788e45..bf9630cd2395 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -507,39 +507,20 @@ int line6_probe(struct usb_interface *interface,
 	int interface_number;
 	int ret;
 
-	/* we don't handle multiple configurations */
-	if (usbdev->descriptor.bNumConfigurations != 1) {
-		ret = -ENODEV;
-		goto err_put;
-	}
-
-	/* initialize device info: */
-	dev_info(&interface->dev, "Line 6 %s found\n", properties->name);
-
-	/* query interface number */
-	interface_number = interface->cur_altsetting->desc.bInterfaceNumber;
-
-	ret = usb_set_interface(usbdev, interface_number,
-			properties->altsetting);
+	ret = snd_card_new(&interface->dev,
+			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
+			   THIS_MODULE, 0, &card);
 	if (ret < 0) {
-		dev_err(&interface->dev, "set_interface failed\n");
-		goto err_put;
+		kfree(line6);
+		return ret;
 	}
 
 	/* store basic data: */
+	line6->card = card;
 	line6->properties = properties;
 	line6->usbdev = usbdev;
 	line6->ifcdev = &interface->dev;
 
-	line6_get_interval(line6);
-
-	ret = snd_card_new(line6->ifcdev,
-			   SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
-			   THIS_MODULE, 0, &card);
-	if (ret < 0)
-		goto err_put;
-
-	line6->card = card;
 	strcpy(card->id, line6->properties->id);
 	strcpy(card->driver, DRIVER_NAME);
 	strcpy(card->shortname, line6->properties->name);
@@ -553,16 +534,37 @@ int line6_probe(struct usb_interface *interface,
 	/* increment reference counters: */
 	usb_get_dev(usbdev);
 
+	/* we don't handle multiple configurations */
+	if (usbdev->descriptor.bNumConfigurations != 1) {
+		ret = -ENODEV;
+		goto error;
+	}
+
+	/* initialize device info: */
+	dev_info(&interface->dev, "Line 6 %s found\n", properties->name);
+
+	/* query interface number */
+	interface_number = interface->cur_altsetting->desc.bInterfaceNumber;
+
+	ret = usb_set_interface(usbdev, interface_number,
+			properties->altsetting);
+	if (ret < 0) {
+		dev_err(&interface->dev, "set_interface failed\n");
+		goto error;
+	}
+
+	line6_get_interval(line6);
+
 	if (properties->capabilities & LINE6_CAP_CONTROL) {
 		ret = line6_init_cap_control(line6);
 		if (ret < 0)
-			goto err_destruct;
+			goto error;
 	}
 
 	/* initialize device data based on device: */
 	ret = private_init(interface, line6);
 	if (ret < 0)
-		goto err_destruct;
+		goto error;
 
 	/* creation of additional special files should go here */
 
@@ -571,11 +573,10 @@ int line6_probe(struct usb_interface *interface,
 
 	return 0;
 
- err_destruct:
+ error:
 	if (line6->disconnect)
 		line6->disconnect(interface);
 	snd_card_free(card);
- err_put:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_probe);
-- 
2.2.2