All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
@ 2022-10-27  1:34 ` Yang Yingliang
  0 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2022-10-27  1:34 UTC (permalink / raw)
  To: alsa-devel, linuxppc-dev; +Cc: johannes, yangyingliang, tiwai

dev_set_name() in soundbus_add_one() allocates memory for name, it need be
freed when of_device_register() fails, call soundbus_dev_put() to give up
the reference that hold in device_initialize(), so that it can be freed in
kobject_cleanup() when the refcount hit to 0. And other resources are also
freed in i2sbus_release_dev(), so it can return 0 directly.

Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 sound/aoa/soundbus/i2sbus/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index faf6b03131ee..f6841daf9e3b 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
 
 	if (soundbus_add_one(&dev->sound)) {
 		printk(KERN_DEBUG "i2sbus: device registration error!\n");
+		if (dev->sound.ofdev.dev.kobj.state_initialized) {
+			soundbus_dev_put(&dev->sound);
+			return 0;
+		}
 		goto err;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
@ 2022-10-27  1:34 ` Yang Yingliang
  0 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2022-10-27  1:34 UTC (permalink / raw)
  To: alsa-devel, linuxppc-dev; +Cc: johannes, yangyingliang, tiwai, perex

dev_set_name() in soundbus_add_one() allocates memory for name, it need be
freed when of_device_register() fails, call soundbus_dev_put() to give up
the reference that hold in device_initialize(), so that it can be freed in
kobject_cleanup() when the refcount hit to 0. And other resources are also
freed in i2sbus_release_dev(), so it can return 0 directly.

Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 sound/aoa/soundbus/i2sbus/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index faf6b03131ee..f6841daf9e3b 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
 
 	if (soundbus_add_one(&dev->sound)) {
 		printk(KERN_DEBUG "i2sbus: device registration error!\n");
+		if (dev->sound.ofdev.dev.kobj.state_initialized) {
+			soundbus_dev_put(&dev->sound);
+			return 0;
+		}
 		goto err;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
  2022-10-27  1:34 ` Yang Yingliang
@ 2022-10-27  6:38   ` Takashi Iwai
  -1 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2022-10-27  6:38 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes

On Thu, 27 Oct 2022 03:34:38 +0200,
Yang Yingliang wrote:
> 
> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
> freed when of_device_register() fails, call soundbus_dev_put() to give up
> the reference that hold in device_initialize(), so that it can be freed in
> kobject_cleanup() when the refcount hit to 0. And other resources are also
> freed in i2sbus_release_dev(), so it can return 0 directly.
> 
> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

The check of kobj state is awkward, but it seems to be the simplest
way...  Applied now.  Thanks!


Takashi


> ---
>  sound/aoa/soundbus/i2sbus/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
> index faf6b03131ee..f6841daf9e3b 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>  
>  	if (soundbus_add_one(&dev->sound)) {
>  		printk(KERN_DEBUG "i2sbus: device registration error!\n");
> +		if (dev->sound.ofdev.dev.kobj.state_initialized) {
> +			soundbus_dev_put(&dev->sound);
> +			return 0;
> +		}
>  		goto err;
>  	}
>  
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
@ 2022-10-27  6:38   ` Takashi Iwai
  0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2022-10-27  6:38 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes, perex

On Thu, 27 Oct 2022 03:34:38 +0200,
Yang Yingliang wrote:
> 
> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
> freed when of_device_register() fails, call soundbus_dev_put() to give up
> the reference that hold in device_initialize(), so that it can be freed in
> kobject_cleanup() when the refcount hit to 0. And other resources are also
> freed in i2sbus_release_dev(), so it can return 0 directly.
> 
> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

The check of kobj state is awkward, but it seems to be the simplest
way...  Applied now.  Thanks!


Takashi


> ---
>  sound/aoa/soundbus/i2sbus/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
> index faf6b03131ee..f6841daf9e3b 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>  
>  	if (soundbus_add_one(&dev->sound)) {
>  		printk(KERN_DEBUG "i2sbus: device registration error!\n");
> +		if (dev->sound.ofdev.dev.kobj.state_initialized) {
> +			soundbus_dev_put(&dev->sound);
> +			return 0;
> +		}
>  		goto err;
>  	}
>  
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
  2022-10-27  6:38   ` Takashi Iwai
@ 2022-10-27  7:41     ` Yang Yingliang
  -1 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2022-10-27  7:41 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes, perex


On 2022/10/27 14:38, Takashi Iwai wrote:
> On Thu, 27 Oct 2022 03:34:38 +0200,
> Yang Yingliang wrote:
>> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
>> freed when of_device_register() fails, call soundbus_dev_put() to give up
>> the reference that hold in device_initialize(), so that it can be freed in
>> kobject_cleanup() when the refcount hit to 0. And other resources are also
>> freed in i2sbus_release_dev(), so it can return 0 directly.
>>
>> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> The check of kobj state is awkward, but it seems to be the simplest
> way...  Applied now.  Thanks!

Indeed, it's awkward, shall we introduce a helper like this:

diff --git a/sound/aoa/soundbus/i2sbus/core.c 
b/sound/aoa/soundbus/i2sbus/core.c
index f6841daf9e3b..950c37e0297e 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -302,7 +302,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,

         if (soundbus_add_one(&dev->sound)) {
                 printk(KERN_DEBUG "i2sbus: device registration error!\n");
-               if (dev->sound.ofdev.dev.kobj.state_initialized) {
+               if (soundbus_dev_initialized(&dev->sound)) {
                         soundbus_dev_put(&dev->sound);
                         return 0;
                 }
diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
index 3a99c1f1a3ca..2c9c95cf156a 100644
--- a/sound/aoa/soundbus/soundbus.h
+++ b/sound/aoa/soundbus/soundbus.h
@@ -174,6 +174,10 @@ struct soundbus_dev {

  extern int soundbus_add_one(struct soundbus_dev *dev);
  extern void soundbus_remove_one(struct soundbus_dev *dev);
+static inline bool soundbus_dev_initialized(struct soundbus_dev *dev)
+{
+       return dev->ofdev.dev.kobj.state_initialized;
+}


Thanks,
Yang
>
>
> Takashi
>
>
>> ---
>>   sound/aoa/soundbus/i2sbus/core.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
>> index faf6b03131ee..f6841daf9e3b 100644
>> --- a/sound/aoa/soundbus/i2sbus/core.c
>> +++ b/sound/aoa/soundbus/i2sbus/core.c
>> @@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>>   
>>   	if (soundbus_add_one(&dev->sound)) {
>>   		printk(KERN_DEBUG "i2sbus: device registration error!\n");
>> +		if (dev->sound.ofdev.dev.kobj.state_initialized) {
>> +			soundbus_dev_put(&dev->sound);
>> +			return 0;
>> +		}
>>   		goto err;
>>   	}
>>   
>> -- 
>> 2.25.1
>>
> .

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
@ 2022-10-27  7:41     ` Yang Yingliang
  0 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2022-10-27  7:41 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes


On 2022/10/27 14:38, Takashi Iwai wrote:
> On Thu, 27 Oct 2022 03:34:38 +0200,
> Yang Yingliang wrote:
>> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
>> freed when of_device_register() fails, call soundbus_dev_put() to give up
>> the reference that hold in device_initialize(), so that it can be freed in
>> kobject_cleanup() when the refcount hit to 0. And other resources are also
>> freed in i2sbus_release_dev(), so it can return 0 directly.
>>
>> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> The check of kobj state is awkward, but it seems to be the simplest
> way...  Applied now.  Thanks!

Indeed, it's awkward, shall we introduce a helper like this:

diff --git a/sound/aoa/soundbus/i2sbus/core.c 
b/sound/aoa/soundbus/i2sbus/core.c
index f6841daf9e3b..950c37e0297e 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -302,7 +302,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,

         if (soundbus_add_one(&dev->sound)) {
                 printk(KERN_DEBUG "i2sbus: device registration error!\n");
-               if (dev->sound.ofdev.dev.kobj.state_initialized) {
+               if (soundbus_dev_initialized(&dev->sound)) {
                         soundbus_dev_put(&dev->sound);
                         return 0;
                 }
diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
index 3a99c1f1a3ca..2c9c95cf156a 100644
--- a/sound/aoa/soundbus/soundbus.h
+++ b/sound/aoa/soundbus/soundbus.h
@@ -174,6 +174,10 @@ struct soundbus_dev {

  extern int soundbus_add_one(struct soundbus_dev *dev);
  extern void soundbus_remove_one(struct soundbus_dev *dev);
+static inline bool soundbus_dev_initialized(struct soundbus_dev *dev)
+{
+       return dev->ofdev.dev.kobj.state_initialized;
+}


Thanks,
Yang
>
>
> Takashi
>
>
>> ---
>>   sound/aoa/soundbus/i2sbus/core.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
>> index faf6b03131ee..f6841daf9e3b 100644
>> --- a/sound/aoa/soundbus/i2sbus/core.c
>> +++ b/sound/aoa/soundbus/i2sbus/core.c
>> @@ -302,6 +302,10 @@ static int i2sbus_add_dev(struct macio_dev *macio,
>>   
>>   	if (soundbus_add_one(&dev->sound)) {
>>   		printk(KERN_DEBUG "i2sbus: device registration error!\n");
>> +		if (dev->sound.ofdev.dev.kobj.state_initialized) {
>> +			soundbus_dev_put(&dev->sound);
>> +			return 0;
>> +		}
>>   		goto err;
>>   	}
>>   
>> -- 
>> 2.25.1
>>
> .

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
  2022-10-27  7:41     ` Yang Yingliang
@ 2022-10-27  8:06       ` Takashi Iwai
  -1 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2022-10-27  8:06 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes

On Thu, 27 Oct 2022 09:41:03 +0200,
Yang Yingliang wrote:
> 
> 
> On 2022/10/27 14:38, Takashi Iwai wrote:
> > On Thu, 27 Oct 2022 03:34:38 +0200,
> > Yang Yingliang wrote:
> >> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
> >> freed when of_device_register() fails, call soundbus_dev_put() to give up
> >> the reference that hold in device_initialize(), so that it can be freed in
> >> kobject_cleanup() when the refcount hit to 0. And other resources are also
> >> freed in i2sbus_release_dev(), so it can return 0 directly.
> >> 
> >> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
> >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > The check of kobj state is awkward, but it seems to be the simplest
> > way...  Applied now.  Thanks!
> 
> Indeed, it's awkward, shall we introduce a helper like this:
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c
> b/sound/aoa/soundbus/i2sbus/core.c
> index f6841daf9e3b..950c37e0297e 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -302,7 +302,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,
> 
>         if (soundbus_add_one(&dev->sound)) {
>                 printk(KERN_DEBUG "i2sbus: device registration error!\n");
> -               if (dev->sound.ofdev.dev.kobj.state_initialized) {
> +               if (soundbus_dev_initialized(&dev->sound)) {
>                         soundbus_dev_put(&dev->sound);
>                         return 0;
>                 }
> diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
> index 3a99c1f1a3ca..2c9c95cf156a 100644
> --- a/sound/aoa/soundbus/soundbus.h
> +++ b/sound/aoa/soundbus/soundbus.h
> @@ -174,6 +174,10 @@ struct soundbus_dev {
> 
>  extern int soundbus_add_one(struct soundbus_dev *dev);
>  extern void soundbus_remove_one(struct soundbus_dev *dev);
> +static inline bool soundbus_dev_initialized(struct soundbus_dev *dev)
> +{
> +       return dev->ofdev.dev.kobj.state_initialized;
> +}

I think it's not worth much as it's used only at a single place.


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev()
@ 2022-10-27  8:06       ` Takashi Iwai
  0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2022-10-27  8:06 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: alsa-devel, tiwai, linuxppc-dev, johannes, perex

On Thu, 27 Oct 2022 09:41:03 +0200,
Yang Yingliang wrote:
> 
> 
> On 2022/10/27 14:38, Takashi Iwai wrote:
> > On Thu, 27 Oct 2022 03:34:38 +0200,
> > Yang Yingliang wrote:
> >> dev_set_name() in soundbus_add_one() allocates memory for name, it need be
> >> freed when of_device_register() fails, call soundbus_dev_put() to give up
> >> the reference that hold in device_initialize(), so that it can be freed in
> >> kobject_cleanup() when the refcount hit to 0. And other resources are also
> >> freed in i2sbus_release_dev(), so it can return 0 directly.
> >> 
> >> Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
> >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > The check of kobj state is awkward, but it seems to be the simplest
> > way...  Applied now.  Thanks!
> 
> Indeed, it's awkward, shall we introduce a helper like this:
> 
> diff --git a/sound/aoa/soundbus/i2sbus/core.c
> b/sound/aoa/soundbus/i2sbus/core.c
> index f6841daf9e3b..950c37e0297e 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -302,7 +302,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,
> 
>         if (soundbus_add_one(&dev->sound)) {
>                 printk(KERN_DEBUG "i2sbus: device registration error!\n");
> -               if (dev->sound.ofdev.dev.kobj.state_initialized) {
> +               if (soundbus_dev_initialized(&dev->sound)) {
>                         soundbus_dev_put(&dev->sound);
>                         return 0;
>                 }
> diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
> index 3a99c1f1a3ca..2c9c95cf156a 100644
> --- a/sound/aoa/soundbus/soundbus.h
> +++ b/sound/aoa/soundbus/soundbus.h
> @@ -174,6 +174,10 @@ struct soundbus_dev {
> 
>  extern int soundbus_add_one(struct soundbus_dev *dev);
>  extern void soundbus_remove_one(struct soundbus_dev *dev);
> +static inline bool soundbus_dev_initialized(struct soundbus_dev *dev)
> +{
> +       return dev->ofdev.dev.kobj.state_initialized;
> +}

I think it's not worth much as it's used only at a single place.


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-27  8:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-27  1:34 [PATCH] ALSA: aoa: i2sbus: fix possible memory leak in i2sbus_add_dev() Yang Yingliang
2022-10-27  1:34 ` Yang Yingliang
2022-10-27  6:38 ` Takashi Iwai
2022-10-27  6:38   ` Takashi Iwai
2022-10-27  7:41   ` Yang Yingliang
2022-10-27  7:41     ` Yang Yingliang
2022-10-27  8:06     ` Takashi Iwai
2022-10-27  8:06       ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.