Re: [PATCH] platform/x86: think-lmi: fix possible memory leak in tlmi_sysfs_init()

[Kurt Borja]

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@xxxxxxxxx>
> ---
>  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@xxxxxxxxx>

-- 
Thanks,
 ~ Kurt