* [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
@ 2021-06-02 3:41 Dongliang Mu
2021-06-02 6:35 ` Dan Carpenter
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dongliang Mu @ 2021-06-02 3:41 UTC (permalink / raw)
To: perex, tiwai, dan.carpenter, alsa-devel, linux-kernel
Cc: Dongliang Mu, syzbot+08a7d8b51ea048a74ffb
The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
to decrease the refcount to zero, which causes device_release never to
be invoked. This leads to memory leak to some resources, like struct
device_private. In addition, we also free some other similar memory
leaks in snd_ctl_led_init/snd_ctl_led_exit.
Fix this by replacing device_del to device_unregister
in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
call kobject_release and delay the release of kobject, which will cause
use-after-free when the memory backing the kobject is freed at once.
Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
v1->v2: fix other similar memory leaks; move kfree(led_card) to a
release function.
sound/core/control_led.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index 25f57c14f294..a90e31dbde61 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -17,6 +17,9 @@ MODULE_LICENSE("GPL");
#define MAX_LED (((SNDRV_CTL_ELEM_ACCESS_MIC_LED - SNDRV_CTL_ELEM_ACCESS_SPK_LED) \
>> SNDRV_CTL_ELEM_ACCESS_LED_SHIFT) + 1)
+#define to_led_card_dev(_dev) \
+ container_of(_dev, struct snd_ctl_led_card, dev)
+
enum snd_ctl_led_mode {
MODE_FOLLOW_MUTE = 0,
MODE_FOLLOW_ROUTE,
@@ -371,6 +374,21 @@ static void snd_ctl_led_disconnect(struct snd_card *card)
snd_ctl_led_refresh();
}
+static void snd_ctl_led_card_release(struct device *dev)
+{
+ struct snd_ctl_led_card *led_card = to_led_card_dev(dev);
+
+ kfree(led_card);
+}
+
+static void snd_ctl_led_release(struct device *dev)
+{
+}
+
+static void snd_ctl_led_dev_release(struct device *dev)
+{
+}
+
/*
* sysfs
*/
@@ -663,6 +681,7 @@ static void snd_ctl_led_sysfs_add(struct snd_card *card)
led_card->number = card->number;
led_card->led = led;
device_initialize(&led_card->dev);
+ led_card->dev.release = snd_ctl_led_card_release;
if (dev_set_name(&led_card->dev, "card%d", card->number) < 0)
goto cerr;
led_card->dev.parent = &led->dev;
@@ -681,7 +700,6 @@ static void snd_ctl_led_sysfs_add(struct snd_card *card)
put_device(&led_card->dev);
cerr2:
printk(KERN_ERR "snd_ctl_led: unable to add card%d", card->number);
- kfree(led_card);
}
}
@@ -700,8 +718,7 @@ static void snd_ctl_led_sysfs_remove(struct snd_card *card)
snprintf(link_name, sizeof(link_name), "led-%s", led->name);
sysfs_remove_link(&card->ctl_dev.kobj, link_name);
sysfs_remove_link(&led_card->dev.kobj, "card");
- device_del(&led_card->dev);
- kfree(led_card);
+ device_unregister(&led_card->dev);
led->cards[card->number] = NULL;
}
}
@@ -723,6 +740,7 @@ static int __init snd_ctl_led_init(void)
device_initialize(&snd_ctl_led_dev);
snd_ctl_led_dev.class = sound_class;
+ snd_ctl_led_dev.release = snd_ctl_led_dev_release;
dev_set_name(&snd_ctl_led_dev, "ctl-led");
if (device_add(&snd_ctl_led_dev)) {
put_device(&snd_ctl_led_dev);
@@ -733,15 +751,16 @@ static int __init snd_ctl_led_init(void)
INIT_LIST_HEAD(&led->controls);
device_initialize(&led->dev);
led->dev.parent = &snd_ctl_led_dev;
+ led->dev.release = snd_ctl_led_release;
led->dev.groups = snd_ctl_led_dev_attr_groups;
dev_set_name(&led->dev, led->name);
if (device_add(&led->dev)) {
put_device(&led->dev);
for (; group > 0; group--) {
led = &snd_ctl_leds[group - 1];
- device_del(&led->dev);
+ device_unregister(&led->dev);
}
- device_del(&snd_ctl_led_dev);
+ device_unregister(&snd_ctl_led_dev);
return -ENOMEM;
}
}
@@ -767,9 +786,9 @@ static void __exit snd_ctl_led_exit(void)
}
for (group = 0; group < MAX_LED; group++) {
led = &snd_ctl_leds[group];
- device_del(&led->dev);
+ device_unregister(&led->dev);
}
- device_del(&snd_ctl_led_dev);
+ device_unregister(&snd_ctl_led_dev);
snd_ctl_led_clean(NULL);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
2021-06-02 3:41 [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register Dongliang Mu
@ 2021-06-02 6:35 ` Dan Carpenter
2021-06-02 6:47 ` Jaroslav Kysela
2021-06-02 6:59 ` Takashi Iwai
2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-06-02 6:35 UTC (permalink / raw)
To: Dongliang Mu
Cc: perex, tiwai, alsa-devel, linux-kernel, syzbot+08a7d8b51ea048a74ffb
On Wed, Jun 02, 2021 at 11:41:36AM +0800, Dongliang Mu wrote:
> The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
> the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
> to decrease the refcount to zero, which causes device_release never to
> be invoked. This leads to memory leak to some resources, like struct
> device_private. In addition, we also free some other similar memory
> leaks in snd_ctl_led_init/snd_ctl_led_exit.
>
> Fix this by replacing device_del to device_unregister
> in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
>
> Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
> call kobject_release and delay the release of kobject, which will cause
> use-after-free when the memory backing the kobject is freed at once.
(Hopefully it's clear to everyone that this is in the original code and
fixed in your patch).
>
> Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
> Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
Looks perfect to me!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
@ 2021-06-02 6:35 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-06-02 6:35 UTC (permalink / raw)
To: Dongliang Mu; +Cc: syzbot+08a7d8b51ea048a74ffb, linux-kernel, alsa-devel, tiwai
On Wed, Jun 02, 2021 at 11:41:36AM +0800, Dongliang Mu wrote:
> The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
> the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
> to decrease the refcount to zero, which causes device_release never to
> be invoked. This leads to memory leak to some resources, like struct
> device_private. In addition, we also free some other similar memory
> leaks in snd_ctl_led_init/snd_ctl_led_exit.
>
> Fix this by replacing device_del to device_unregister
> in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
>
> Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
> call kobject_release and delay the release of kobject, which will cause
> use-after-free when the memory backing the kobject is freed at once.
(Hopefully it's clear to everyone that this is in the original code and
fixed in your patch).
>
> Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
> Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
Looks perfect to me!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
2021-06-02 3:41 [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register Dongliang Mu
2021-06-02 6:35 ` Dan Carpenter
@ 2021-06-02 6:47 ` Jaroslav Kysela
2021-06-02 6:59 ` Takashi Iwai
2 siblings, 0 replies; 6+ messages in thread
From: Jaroslav Kysela @ 2021-06-02 6:47 UTC (permalink / raw)
To: Dongliang Mu, tiwai, dan.carpenter, alsa-devel, linux-kernel
Cc: syzbot+08a7d8b51ea048a74ffb
On 02. 06. 21 5:41, Dongliang Mu wrote:
> The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
> the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
> to decrease the refcount to zero, which causes device_release never to
> be invoked. This leads to memory leak to some resources, like struct
> device_private. In addition, we also free some other similar memory
> leaks in snd_ctl_led_init/snd_ctl_led_exit.
>
> Fix this by replacing device_del to device_unregister
> in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
>
> Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
> call kobject_release and delay the release of kobject, which will cause
> use-after-free when the memory backing the kobject is freed at once.
>
> Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
> Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
Many thanks to you and Dan to cover this.
Revieved-by: Jaroslav Kysela <perex@perex.cz>
--
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
2021-06-02 3:41 [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register Dongliang Mu
@ 2021-06-02 6:59 ` Takashi Iwai
2021-06-02 6:47 ` Jaroslav Kysela
2021-06-02 6:59 ` Takashi Iwai
2 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-06-02 6:59 UTC (permalink / raw)
To: Dongliang Mu
Cc: perex, tiwai, dan.carpenter, alsa-devel, linux-kernel,
syzbot+08a7d8b51ea048a74ffb
On Wed, 02 Jun 2021 05:41:36 +0200,
Dongliang Mu wrote:
>
> The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
> the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
> to decrease the refcount to zero, which causes device_release never to
> be invoked. This leads to memory leak to some resources, like struct
> device_private. In addition, we also free some other similar memory
> leaks in snd_ctl_led_init/snd_ctl_led_exit.
>
> Fix this by replacing device_del to device_unregister
> in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
>
> Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
> call kobject_release and delay the release of kobject, which will cause
> use-after-free when the memory backing the kobject is freed at once.
>
> Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
> Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
Applied now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register
@ 2021-06-02 6:59 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-06-02 6:59 UTC (permalink / raw)
To: Dongliang Mu
Cc: alsa-devel, tiwai, syzbot+08a7d8b51ea048a74ffb, linux-kernel,
dan.carpenter
On Wed, 02 Jun 2021 05:41:36 +0200,
Dongliang Mu wrote:
>
> The snd_ctl_led_sysfs_add and snd_ctl_led_sysfs_remove should contain
> the refcount operations in pair. However, snd_ctl_led_sysfs_remove fails
> to decrease the refcount to zero, which causes device_release never to
> be invoked. This leads to memory leak to some resources, like struct
> device_private. In addition, we also free some other similar memory
> leaks in snd_ctl_led_init/snd_ctl_led_exit.
>
> Fix this by replacing device_del to device_unregister
> in snd_ctl_led_sysfs_remove/snd_ctl_led_init/snd_ctl_led_exit.
>
> Note that, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled, put_device will
> call kobject_release and delay the release of kobject, which will cause
> use-after-free when the memory backing the kobject is freed at once.
>
> Reported-by: syzbot+08a7d8b51ea048a74ffb@syzkaller.appspotmail.com
> Fixes: a135dfb5de1 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
Applied now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-02 7:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 3:41 [PATCH v2] ALSA: control led: fix memory leak in snd_ctl_led_register Dongliang Mu
2021-06-02 6:35 ` Dan Carpenter
2021-06-02 6:35 ` Dan Carpenter
2021-06-02 6:47 ` Jaroslav Kysela
2021-06-02 6:59 ` Takashi Iwai
2021-06-02 6:59 ` 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.