* [PATCH] cpuidle: Fix memory leaks
@ 2021-09-06 18:34 Anel Orazgaliyeva
2021-09-15 12:14 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Anel Orazgaliyeva @ 2021-09-06 18:34 UTC (permalink / raw)
Cc: anelkz, Aman Priyadarshi, Rafael J. Wysocki, Daniel Lezcano,
linux-pm, linux-kernel
Commit c343bf1ba5ef ("cpuidle: Fix three reference count leaks")
fixes the cleanup of kobjects; however, it removes kfree() calls
altogether, leading to memory leaks.
Fix those and also defer the initialization of dev->kobj_dev until
after the error check, so that we do not end up with a dangling
pointer.
Signed-off-by: Anel Orazgaliyeva <anelkz@amazon.de>
Suggested-by: Aman Priyadarshi <apeureka@amazon.de>
---
drivers/cpuidle/sysfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 53ec9585ccd4..469e18547d06 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -488,6 +488,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
&kdev->kobj, "state%d", i);
if (ret) {
kobject_put(&kobj->kobj);
+ kfree(kobj);
goto error_state;
}
cpuidle_add_s2idle_attr_group(kobj);
@@ -619,6 +620,7 @@ static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev)
&kdev->kobj, "driver");
if (ret) {
kobject_put(&kdrv->kobj);
+ kfree(kdrv);
return ret;
}
@@ -705,7 +707,6 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev)
if (!kdev)
return -ENOMEM;
kdev->dev = dev;
- dev->kobj_dev = kdev;
init_completion(&kdev->kobj_unregister);
@@ -713,9 +714,11 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev)
"cpuidle");
if (error) {
kobject_put(&kdev->kobj);
+ kfree(kdev);
return error;
}
+ dev->kobj_dev = kdev;
kobject_uevent(&kdev->kobj, KOBJ_ADD);
return 0;
--
2.32.0
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] cpuidle: Fix memory leaks 2021-09-06 18:34 [PATCH] cpuidle: Fix memory leaks Anel Orazgaliyeva @ 2021-09-15 12:14 ` Rafael J. Wysocki 2021-09-15 15:44 ` Anel Orazgaliyeva 0 siblings, 1 reply; 4+ messages in thread From: Rafael J. Wysocki @ 2021-09-15 12:14 UTC (permalink / raw) To: Anel Orazgaliyeva Cc: Aman Priyadarshi, Rafael J. Wysocki, Daniel Lezcano, Linux PM, Linux Kernel Mailing List On Mon, Sep 6, 2021 at 8:35 PM Anel Orazgaliyeva <anelkz@amazon.de> wrote: > > Commit c343bf1ba5ef ("cpuidle: Fix three reference count leaks") > fixes the cleanup of kobjects; however, it removes kfree() calls > altogether, leading to memory leaks. Wait, won't the cleanup be done by cpuidle_free_state_kobj()? > Fix those and also defer the initialization of dev->kobj_dev until > after the error check, so that we do not end up with a dangling > pointer. > > Signed-off-by: Anel Orazgaliyeva <anelkz@amazon.de> > Suggested-by: Aman Priyadarshi <apeureka@amazon.de> > --- > drivers/cpuidle/sysfs.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c > index 53ec9585ccd4..469e18547d06 100644 > --- a/drivers/cpuidle/sysfs.c > +++ b/drivers/cpuidle/sysfs.c > @@ -488,6 +488,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) > &kdev->kobj, "state%d", i); > if (ret) { > kobject_put(&kobj->kobj); > + kfree(kobj); > goto error_state; > } > cpuidle_add_s2idle_attr_group(kobj); > @@ -619,6 +620,7 @@ static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev) > &kdev->kobj, "driver"); > if (ret) { > kobject_put(&kdrv->kobj); > + kfree(kdrv); > return ret; > } > > @@ -705,7 +707,6 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > if (!kdev) > return -ENOMEM; > kdev->dev = dev; > - dev->kobj_dev = kdev; > > init_completion(&kdev->kobj_unregister); > > @@ -713,9 +714,11 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > "cpuidle"); > if (error) { > kobject_put(&kdev->kobj); > + kfree(kdev); > return error; > } > > + dev->kobj_dev = kdev; > kobject_uevent(&kdev->kobj, KOBJ_ADD); > > return 0; > -- > 2.32.0 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpuidle: Fix memory leaks 2021-09-15 12:14 ` Rafael J. Wysocki @ 2021-09-15 15:44 ` Anel Orazgaliyeva 2021-10-01 18:45 ` Rafael J. Wysocki 0 siblings, 1 reply; 4+ messages in thread From: Anel Orazgaliyeva @ 2021-09-15 15:44 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Aman Priyadarshi, Daniel Lezcano, linux-pm, linux-kernel > On Wed, Sep 15, 2021 at 02:14:56PM +0200, Rafael J. Wysocki wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > On Mon, Sep 6, 2021 at 8:35 PM Anel Orazgaliyeva <anelkz@amazon.de> wrote: > > > > Commit c343bf1ba5ef ("cpuidle: Fix three reference count leaks") > > fixes the cleanup of kobjects; however, it removes kfree() calls > > altogether, leading to memory leaks. > > Wait, won't the cleanup be done by cpuidle_free_state_kobj()? For state cleanup, cpuidle_free_state_kobj() is called on the all the previously created kobjs: error_state: for (i = i - 1; i >= 0; i--) cpuidle_free_state_kobj(device, i); so we still need to cleanup the kobj created in the current iteration. For overall sysfs, the flow is as follows: cpuidle_register_device ret = cpuidle_add_sysfs(dev); if (ret) goto out_unregister; out_unregister: __cpuidle_unregister_device(dev); goto out_unlock; so when there is an error in cpuidle_add_sysfs() dev doesn’t get freed. > > > Fix those and also defer the initialization of dev->kobj_dev until > > after the error check, so that we do not end up with a dangling > > pointer. > > > > Signed-off-by: Anel Orazgaliyeva <anelkz@amazon.de> > > Suggested-by: Aman Priyadarshi <apeureka@amazon.de> > > --- > > drivers/cpuidle/sysfs.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c > > index 53ec9585ccd4..469e18547d06 100644 > > --- a/drivers/cpuidle/sysfs.c > > +++ b/drivers/cpuidle/sysfs.c > > @@ -488,6 +488,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) > > &kdev->kobj, "state%d", i); > > if (ret) { > > kobject_put(&kobj->kobj); > > + kfree(kobj); > > goto error_state; > > } > > cpuidle_add_s2idle_attr_group(kobj); > > @@ -619,6 +620,7 @@ static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev) > > &kdev->kobj, "driver"); > > if (ret) { > > kobject_put(&kdrv->kobj); > > + kfree(kdrv); > > return ret; > > } > > > > @@ -705,7 +707,6 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > > if (!kdev) > > return -ENOMEM; > > kdev->dev = dev; > > - dev->kobj_dev = kdev; > > > > init_completion(&kdev->kobj_unregister); > > > > @@ -713,9 +714,11 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > > "cpuidle"); > > if (error) { > > kobject_put(&kdev->kobj); > > + kfree(kdev); > > return error; > > } > > > > + dev->kobj_dev = kdev; > > kobject_uevent(&kdev->kobj, KOBJ_ADD); > > > > return 0; > > -- > > 2.32.0 > > > > > > > > > > Amazon Development Center Germany GmbH > > Krausenstr. 38 > > 10117 Berlin > > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > > Sitz: Berlin > > Ust-ID: DE 289 237 879 On Wed, Sep 15, 2021 at 02:14:56PM +0200, Rafael J. Wysocki wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > On Mon, Sep 6, 2021 at 8:35 PM Anel Orazgaliyeva <anelkz@amazon.de> wrote: > > > > Commit c343bf1ba5ef ("cpuidle: Fix three reference count leaks") > > fixes the cleanup of kobjects; however, it removes kfree() calls > > altogether, leading to memory leaks. > > Wait, won't the cleanup be done by cpuidle_free_state_kobj()? > > > Fix those and also defer the initialization of dev->kobj_dev until > > after the error check, so that we do not end up with a dangling > > pointer. > > > > Signed-off-by: Anel Orazgaliyeva <anelkz@amazon.de> > > Suggested-by: Aman Priyadarshi <apeureka@amazon.de> > > --- > > drivers/cpuidle/sysfs.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c > > index 53ec9585ccd4..469e18547d06 100644 > > --- a/drivers/cpuidle/sysfs.c > > +++ b/drivers/cpuidle/sysfs.c > > @@ -488,6 +488,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device) > > &kdev->kobj, "state%d", i); > > if (ret) { > > kobject_put(&kobj->kobj); > > + kfree(kobj); > > goto error_state; > > } > > cpuidle_add_s2idle_attr_group(kobj); > > @@ -619,6 +620,7 @@ static int cpuidle_add_driver_sysfs(struct cpuidle_device *dev) > > &kdev->kobj, "driver"); > > if (ret) { > > kobject_put(&kdrv->kobj); > > + kfree(kdrv); > > return ret; > > } > > > > @@ -705,7 +707,6 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > > if (!kdev) > > return -ENOMEM; > > kdev->dev = dev; > > - dev->kobj_dev = kdev; > > > > init_completion(&kdev->kobj_unregister); > > > > @@ -713,9 +714,11 @@ int cpuidle_add_sysfs(struct cpuidle_device *dev) > > "cpuidle"); > > if (error) { > > kobject_put(&kdev->kobj); > > + kfree(kdev); > > return error; > > } > > > > + dev->kobj_dev = kdev; > > kobject_uevent(&kdev->kobj, KOBJ_ADD); > > > > return 0; > > -- > > 2.32.0 > > > > > > > > > > Amazon Development Center Germany GmbH > > Krausenstr. 38 > > 10117 Berlin > > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > > Sitz: Berlin > > Ust-ID: DE 289 237 879 > > > > > > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpuidle: Fix memory leaks 2021-09-15 15:44 ` Anel Orazgaliyeva @ 2021-10-01 18:45 ` Rafael J. Wysocki 0 siblings, 0 replies; 4+ messages in thread From: Rafael J. Wysocki @ 2021-10-01 18:45 UTC (permalink / raw) To: Anel Orazgaliyeva Cc: Rafael J. Wysocki, Aman Priyadarshi, Daniel Lezcano, Linux PM, Linux Kernel Mailing List On Wed, Sep 15, 2021 at 5:45 PM Anel Orazgaliyeva <anelkz@amazon.de> wrote: > > > On Wed, Sep 15, 2021 at 02:14:56PM +0200, Rafael J. Wysocki wrote: > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > > > > > On Mon, Sep 6, 2021 at 8:35 PM Anel Orazgaliyeva <anelkz@amazon.de> wrote: > > > > > > Commit c343bf1ba5ef ("cpuidle: Fix three reference count leaks") > > > fixes the cleanup of kobjects; however, it removes kfree() calls > > > altogether, leading to memory leaks. > > > > Wait, won't the cleanup be done by cpuidle_free_state_kobj()? > > For state cleanup, cpuidle_free_state_kobj() is called on the all the previously created kobjs: > > error_state: > for (i = i - 1; i >= 0; i--) > cpuidle_free_state_kobj(device, i); > > so we still need to cleanup the kobj created in the current iteration. > > > For overall sysfs, the flow is as follows: > > cpuidle_register_device > ret = cpuidle_add_sysfs(dev); > if (ret) > goto out_unregister; > > out_unregister: > __cpuidle_unregister_device(dev); > goto out_unlock; > > so when there is an error in cpuidle_add_sysfs() dev doesn’t get freed. OK, applied as 5.16 material, thanks! ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-01 18:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-06 18:34 [PATCH] cpuidle: Fix memory leaks Anel Orazgaliyeva 2021-09-15 12:14 ` Rafael J. Wysocki 2021-09-15 15:44 ` Anel Orazgaliyeva 2021-10-01 18:45 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).