[PATCH] ALSA: hda - Fix memory leak on snd_hdac_device_init error

[PATCH] ALSA: hda - Fix memory leak on snd_hdac_device_init error

[Guneshwor Singh]

From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>

Free codec vendor name on snd_hdac_device_init error.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
---
 sound/hda/hdac_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index 19deb306facb..467b949544e2 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -111,6 +111,8 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
 	return 0;
 
  error:
+	kfree(codec->vendor_name);
+
 	put_device(&codec->dev);
 	return err;
 }
-- 
2.15.0

Re: [PATCH] ALSA: hda - Fix memory leak on snd_hdac_device_init error

[Vinod Koul]

On Thu, Nov 09, 2017 at 08:57:51AM +0530, Guneshwor Singh wrote:
> From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
> 
> Free codec vendor name on snd_hdac_device_init error.
> 
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
> ---
>  sound/hda/hdac_device.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> index 19deb306facb..467b949544e2 100644
> --- a/sound/hda/hdac_device.c
> +++ b/sound/hda/hdac_device.c
> @@ -111,6 +111,8 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
>  	return 0;
>  
>   error:
> +	kfree(codec->vendor_name);

Sorry I dont think this is right.

The vendor name is freed in the snd_hdac_device_exit()

We drop the reference here and release method would be called to
free up the resources.

>  	put_device(&codec->dev);
>  	return err;
>  }
> -- 
> 2.15.0
> 

-- 
~Vinod

Re: [PATCH] ALSA: hda - Fix memory leak on snd_hdac_device_init error

[Takashi Iwai]

On Thu, 09 Nov 2017 05:44:19 +0100,
Vinod Koul wrote:
> 
> On Thu, Nov 09, 2017 at 08:57:51AM +0530, Guneshwor Singh wrote:
> > From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
> > 
> > Free codec vendor name on snd_hdac_device_init error.
> > 
> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
> > ---
> >  sound/hda/hdac_device.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> > index 19deb306facb..467b949544e2 100644
> > --- a/sound/hda/hdac_device.c
> > +++ b/sound/hda/hdac_device.c
> > @@ -111,6 +111,8 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
> >  	return 0;
> >  
> >   error:
> > +	kfree(codec->vendor_name);
> 
> Sorry I dont think this is right.
> 
> The vendor name is freed in the snd_hdac_device_exit()
> 
> We drop the reference here and release method would be called to
> free up the resources.

Correct.  The patch would lead to a double-free.


thanks,

Takashi


> 
> >  	put_device(&codec->dev);
> >  	return err;
> >  }
> > -- 
> > 2.15.0
> > 
> 
> -- 
> ~Vinod
> 

Re: [PATCH] ALSA: hda - Fix memory leak on snd_hdac_device_init error

[Singh, Guneshwor]

On Thu, Nov 09, 2017 at 08:31:56AM +0100, Takashi Iwai wrote:
> On Thu, 09 Nov 2017 05:44:19 +0100,
> Vinod Koul wrote:
> > 
> > On Thu, Nov 09, 2017 at 08:57:51AM +0530, Guneshwor Singh wrote:
> > > From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
> > > 
> > > Free codec vendor name on snd_hdac_device_init error.
> > > 
> > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > > Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
> > > ---
> > >  sound/hda/hdac_device.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> > > index 19deb306facb..467b949544e2 100644
> > > --- a/sound/hda/hdac_device.c
> > > +++ b/sound/hda/hdac_device.c
> > > @@ -111,6 +111,8 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
> > >  	return 0;
> > >  
> > >   error:
> > > +	kfree(codec->vendor_name);
> > 
> > Sorry I dont think this is right.
> > 
> > The vendor name is freed in the snd_hdac_device_exit()
> > 
> > We drop the reference here and release method would be called to
> > free up the resources.
> 
> Correct.  The patch would lead to a double-free.
> 
> 

Yes, you are right. Please ignore the patch.

> thanks,
> 
> Takashi
> 
> 
> > 
> > >  	put_device(&codec->dev);
> > >  	return err;
> > >  }
> > > -- 
> > > 2.15.0
> > > 
> > 
> > -- 
> > ~Vinod
> >