* [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init()
@ 2026-04-28 8:22 Guangshuo Li
2026-04-28 13:52 ` Mark Pearson
2026-04-28 14:10 ` Kurt Borja
0 siblings, 2 replies; 3+ messages in thread
From: Guangshuo Li @ 2026-04-28 8:22 UTC (permalink / raw)
To: Mark Pearson, Derek J. Clark, Hans de Goede, Ilpo Järvinen,
Kurt Borja, platform-driver-x86, linux-kernel
Cc: Guangshuo Li
Once kobject_init_and_add() fails, kobject_put() should be called to
decrement the reference count for cleanup. Otherwise, the memory associated
with the object may leak.
tlmi_sysfs_init() jumps to fail_create_attr after kobject_init_and_add()
fails. The error path calls tlmi_release_attr(), which walks the kset
lists and puts the kobjects found there. However, when the add operation
fails, the kobject core removes the failed object from the kset list
before returning the error. Therefore tlmi_release_attr() cannot put the
kobject whose kobject_init_and_add() failed.
Fix this by calling kobject_put() for the failed kobject before jumping to
the common error path. Since pwd_admin's saved signatures are released
outside of its kobject release callback, release them before putting a
failed pwd_admin object.
This issue was found by a static analysis tool I am developing.
Fixes: 9110056fe10b ("platform/x86: think-lmi: Fix kobject cleanup")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/platform/x86/lenovo/think-lmi.c | 40 ++++++++++++++++++++-----
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
index e215e86e3db7..994475eb0b9c 100644
--- a/drivers/platform/x86/lenovo/think-lmi.c
+++ b/drivers/platform/x86/lenovo/think-lmi.c
@@ -1454,8 +1454,10 @@ static void tlmi_release_attr(void)
kset_unregister(tlmi_priv.attribute_kset);
/* Free up any saved signatures */
- kfree(tlmi_priv.pwd_admin->signature);
- kfree(tlmi_priv.pwd_admin->save_signature);
+ if (tlmi_priv.pwd_admin) {
+ kfree(tlmi_priv.pwd_admin->signature);
+ kfree(tlmi_priv.pwd_admin->save_signature);
+ }
/* Authentication structures */
list_for_each_entry_safe(pos, n, &tlmi_priv.authentication_kset->list, entry)
@@ -1526,8 +1528,11 @@ static int tlmi_sysfs_init(void)
tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj, &tlmi_attr_setting_ktype,
NULL, "%s", tlmi_priv.setting[i]->display_name);
- if (ret)
+ if (ret) {
+ kobject_put(&tlmi_priv.setting[i]->kobj);
+ tlmi_priv.setting[i] = NULL;
goto fail_create_attr;
+ }
}
ret = sysfs_create_file(&tlmi_priv.attribute_kset->kobj, &pending_reboot.attr);
@@ -1548,33 +1553,52 @@ static int tlmi_sysfs_init(void)
tlmi_priv.pwd_admin->kobj.kset = tlmi_priv.authentication_kset;
ret = kobject_init_and_add(&tlmi_priv.pwd_admin->kobj, &tlmi_pwd_setting_ktype,
NULL, "%s", "Admin");
- if (ret)
+ if (ret) {
+ kfree(tlmi_priv.pwd_admin->signature);
+ kfree(tlmi_priv.pwd_admin->save_signature);
+ tlmi_priv.pwd_admin->signature = NULL;
+ tlmi_priv.pwd_admin->save_signature = NULL;
+ kobject_put(&tlmi_priv.pwd_admin->kobj);
+ tlmi_priv.pwd_admin = NULL;
goto fail_create_attr;
+ }
tlmi_priv.pwd_power->kobj.kset = tlmi_priv.authentication_kset;
ret = kobject_init_and_add(&tlmi_priv.pwd_power->kobj, &tlmi_pwd_setting_ktype,
NULL, "%s", "Power-on");
- if (ret)
+ if (ret) {
+ kobject_put(&tlmi_priv.pwd_power->kobj);
+ tlmi_priv.pwd_power = NULL;
goto fail_create_attr;
+ }
if (tlmi_priv.opcode_support) {
tlmi_priv.pwd_system->kobj.kset = tlmi_priv.authentication_kset;
ret = kobject_init_and_add(&tlmi_priv.pwd_system->kobj, &tlmi_pwd_setting_ktype,
NULL, "%s", "System");
- if (ret)
+ if (ret) {
+ kobject_put(&tlmi_priv.pwd_system->kobj);
+ tlmi_priv.pwd_system = NULL;
goto fail_create_attr;
+ }
tlmi_priv.pwd_hdd->kobj.kset = tlmi_priv.authentication_kset;
ret = kobject_init_and_add(&tlmi_priv.pwd_hdd->kobj, &tlmi_pwd_setting_ktype,
NULL, "%s", "HDD");
- if (ret)
+ if (ret) {
+ kobject_put(&tlmi_priv.pwd_hdd->kobj);
+ tlmi_priv.pwd_hdd = NULL;
goto fail_create_attr;
+ }
tlmi_priv.pwd_nvme->kobj.kset = tlmi_priv.authentication_kset;
ret = kobject_init_and_add(&tlmi_priv.pwd_nvme->kobj, &tlmi_pwd_setting_ktype,
NULL, "%s", "NVMe");
- if (ret)
+ if (ret) {
+ kobject_put(&tlmi_priv.pwd_nvme->kobj);
+ tlmi_priv.pwd_nvme = NULL;
goto fail_create_attr;
+ }
}
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init()
2026-04-28 8:22 [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init() Guangshuo Li
@ 2026-04-28 13:52 ` Mark Pearson
2026-04-28 14:10 ` Kurt Borja
1 sibling, 0 replies; 3+ messages in thread
From: Mark Pearson @ 2026-04-28 13:52 UTC (permalink / raw)
To: Guangshuo Li, Derek J . Clark, Hans de Goede, Ilpo Järvinen,
Kurt Borja, platform-driver-x86@vger.kernel.org, linux-kernel
Thanks Guangshuo,
On Tue, Apr 28, 2026, at 4:22 AM, Guangshuo Li wrote:
> Once kobject_init_and_add() fails, kobject_put() should be called to
> decrement the reference count for cleanup. Otherwise, the memory associated
> with the object may leak.
>
> tlmi_sysfs_init() jumps to fail_create_attr after kobject_init_and_add()
> fails. The error path calls tlmi_release_attr(), which walks the kset
> lists and puts the kobjects found there. However, when the add operation
> fails, the kobject core removes the failed object from the kset list
> before returning the error. Therefore tlmi_release_attr() cannot put the
> kobject whose kobject_init_and_add() failed.
>
> Fix this by calling kobject_put() for the failed kobject before jumping to
> the common error path. Since pwd_admin's saved signatures are released
> outside of its kobject release callback, release them before putting a
> failed pwd_admin object.
>
> This issue was found by a static analysis tool I am developing.
>
> Fixes: 9110056fe10b ("platform/x86: think-lmi: Fix kobject cleanup")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/platform/x86/lenovo/think-lmi.c | 40 ++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/think-lmi.c
> b/drivers/platform/x86/lenovo/think-lmi.c
> index e215e86e3db7..994475eb0b9c 100644
> --- a/drivers/platform/x86/lenovo/think-lmi.c
> +++ b/drivers/platform/x86/lenovo/think-lmi.c
> @@ -1454,8 +1454,10 @@ static void tlmi_release_attr(void)
> kset_unregister(tlmi_priv.attribute_kset);
>
> /* Free up any saved signatures */
> - kfree(tlmi_priv.pwd_admin->signature);
> - kfree(tlmi_priv.pwd_admin->save_signature);
> + if (tlmi_priv.pwd_admin) {
> + kfree(tlmi_priv.pwd_admin->signature);
> + kfree(tlmi_priv.pwd_admin->save_signature);
> + }
>
> /* Authentication structures */
> list_for_each_entry_safe(pos, n,
> &tlmi_priv.authentication_kset->list, entry)
> @@ -1526,8 +1528,11 @@ static int tlmi_sysfs_init(void)
> tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
> ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj,
> &tlmi_attr_setting_ktype,
> NULL, "%s", tlmi_priv.setting[i]->display_name);
> - if (ret)
> + if (ret) {
> + kobject_put(&tlmi_priv.setting[i]->kobj);
> + tlmi_priv.setting[i] = NULL;
> goto fail_create_attr;
> + }
> }
>
> ret = sysfs_create_file(&tlmi_priv.attribute_kset->kobj,
> &pending_reboot.attr);
> @@ -1548,33 +1553,52 @@ static int tlmi_sysfs_init(void)
> tlmi_priv.pwd_admin->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_admin->kobj,
> &tlmi_pwd_setting_ktype,
> NULL, "%s", "Admin");
> - if (ret)
> + if (ret) {
> + kfree(tlmi_priv.pwd_admin->signature);
> + kfree(tlmi_priv.pwd_admin->save_signature);
> + tlmi_priv.pwd_admin->signature = NULL;
> + tlmi_priv.pwd_admin->save_signature = NULL;
> + kobject_put(&tlmi_priv.pwd_admin->kobj);
> + tlmi_priv.pwd_admin = NULL;
> goto fail_create_attr;
> + }
>
> tlmi_priv.pwd_power->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_power->kobj,
> &tlmi_pwd_setting_ktype,
> NULL, "%s", "Power-on");
> - if (ret)
> + if (ret) {
> + kobject_put(&tlmi_priv.pwd_power->kobj);
> + tlmi_priv.pwd_power = NULL;
> goto fail_create_attr;
> + }
>
> if (tlmi_priv.opcode_support) {
> tlmi_priv.pwd_system->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_system->kobj,
> &tlmi_pwd_setting_ktype,
> NULL, "%s", "System");
> - if (ret)
> + if (ret) {
> + kobject_put(&tlmi_priv.pwd_system->kobj);
> + tlmi_priv.pwd_system = NULL;
> goto fail_create_attr;
> + }
>
> tlmi_priv.pwd_hdd->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_hdd->kobj, &tlmi_pwd_setting_ktype,
> NULL, "%s", "HDD");
> - if (ret)
> + if (ret) {
> + kobject_put(&tlmi_priv.pwd_hdd->kobj);
> + tlmi_priv.pwd_hdd = NULL;
> goto fail_create_attr;
> + }
>
> tlmi_priv.pwd_nvme->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_nvme->kobj,
> &tlmi_pwd_setting_ktype,
> NULL, "%s", "NVMe");
> - if (ret)
> + if (ret) {
> + kobject_put(&tlmi_priv.pwd_nvme->kobj);
> + tlmi_priv.pwd_nvme = NULL;
> goto fail_create_attr;
> + }
> }
>
> return ret;
> --
> 2.43.0
This looks good to me (good find).
Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init()
2026-04-28 8:22 [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init() Guangshuo Li
2026-04-28 13:52 ` Mark Pearson
@ 2026-04-28 14:10 ` Kurt Borja
1 sibling, 0 replies; 3+ messages in thread
From: Kurt Borja @ 2026-04-28 14:10 UTC (permalink / raw)
To: Guangshuo Li, Mark Pearson, Derek J. Clark, Hans de Goede,
Ilpo Järvinen, Kurt Borja, platform-driver-x86, linux-kernel
Hi Guangshuo,
On Tue Apr 28, 2026 at 3:22 AM -05, Guangshuo Li wrote:
> Once kobject_init_and_add() fails, kobject_put() should be called to
> decrement the reference count for cleanup. Otherwise, the memory associated
> with the object may leak.
>
> tlmi_sysfs_init() jumps to fail_create_attr after kobject_init_and_add()
> fails. The error path calls tlmi_release_attr(), which walks the kset
> lists and puts the kobjects found there. However, when the add operation
> fails, the kobject core removes the failed object from the kset list
> before returning the error. Therefore tlmi_release_attr() cannot put the
> kobject whose kobject_init_and_add() failed.
>
> Fix this by calling kobject_put() for the failed kobject before jumping to
> the common error path. Since pwd_admin's saved signatures are released
> outside of its kobject release callback, release them before putting a
> failed pwd_admin object.
>
> This issue was found by a static analysis tool I am developing.
Thanks!
Just a comment bellow.
>
> Fixes: 9110056fe10b ("platform/x86: think-lmi: Fix kobject cleanup")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/platform/x86/lenovo/think-lmi.c | 40 ++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
> index e215e86e3db7..994475eb0b9c 100644
> --- a/drivers/platform/x86/lenovo/think-lmi.c
> +++ b/drivers/platform/x86/lenovo/think-lmi.c
...
> @@ -1548,33 +1553,52 @@ static int tlmi_sysfs_init(void)
> tlmi_priv.pwd_admin->kobj.kset = tlmi_priv.authentication_kset;
> ret = kobject_init_and_add(&tlmi_priv.pwd_admin->kobj, &tlmi_pwd_setting_ktype,
> NULL, "%s", "Admin");
> - if (ret)
> + if (ret) {
> + kfree(tlmi_priv.pwd_admin->signature);
> + kfree(tlmi_priv.pwd_admin->save_signature);
> + tlmi_priv.pwd_admin->signature = NULL;
> + tlmi_priv.pwd_admin->save_signature = NULL;
I think, up until now (in the probe path that leads to this), we have
not yet allocated signature and save_signature. These are allocated and
reallocated once the sysfs files are created.
Other than that, this looks good.
Reviewed-by: Kurt Borja <kuurtb@gmail.com>
--
Thanks,
~ Kurt
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-28 14:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 8:22 [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init() Guangshuo Li
2026-04-28 13:52 ` Mark Pearson
2026-04-28 14:10 ` Kurt Borja
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox