[PATCH 1/3] platform/x86: lenovo-wmi-helpers: Fix memory leak in lwmi_dev_evaluate_int()

[PATCH 1/3] platform/x86: lenovo-wmi-helpers: Fix memory leak in lwmi_dev_evaluate_int()

[Rong Zhang]

lwmi_dev_evaluate_int() leaks output.pointer when retval == NULL (found
by sashiko.dev [1]).

Fix it by moving `ret_obj = output.pointer' outside of the `if (retval)'
block so that it is always freed by the __free cleanup callback.

No functional change intended.

Fixes: e521d16e76cd ("platform/x86: Add lenovo-wmi-helpers")
Cc: stable@vger.kernel.org
Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-helpers.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo/wmi-helpers.c b/drivers/platform/x86/lenovo/wmi-helpers.c
index 7379defac500..80021f59d1ef 100644
--- a/drivers/platform/x86/lenovo/wmi-helpers.c
+++ b/drivers/platform/x86/lenovo/wmi-helpers.c
@@ -55,8 +55,9 @@ int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
+	ret_obj = output.pointer;
+
 	if (retval) {
-		ret_obj = output.pointer;
 		if (!ret_obj)
 			return -ENODATA;
 

base-commit: 9147566d801602c9e7fc7f85e989735735bf38ba
-- 
2.53.0

Re: [PATCH 1/3] platform/x86: lenovo-wmi-helpers: Fix memory leak in lwmi_dev_evaluate_int()

[Ilpo Järvinen]

On Thu, 2 Apr 2026, Rong Zhang wrote:

> lwmi_dev_evaluate_int() leaks output.pointer when retval == NULL (found
> by sashiko.dev [1]).
> 
> Fix it by moving `ret_obj = output.pointer' outside of the `if (retval)'
> block so that it is always freed by the __free cleanup callback.
> 
> No functional change intended.
> 
> Fixes: e521d16e76cd ("platform/x86: Add lenovo-wmi-helpers")
> Cc: stable@vger.kernel.org
> Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/wmi-helpers.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-helpers.c b/drivers/platform/x86/lenovo/wmi-helpers.c
> index 7379defac500..80021f59d1ef 100644
> --- a/drivers/platform/x86/lenovo/wmi-helpers.c
> +++ b/drivers/platform/x86/lenovo/wmi-helpers.c
> @@ -55,8 +55,9 @@ int lwmi_dev_evaluate_int(struct wmi_device *wdev, u8 instance, u32 method_id,
>  	if (ACPI_FAILURE(status))
>  		return -EIO;
>  
> +	ret_obj = output.pointer;

To follow the best practice, could you please also place the variable 
declaration here as well so there's no separate ret_obj = NULL line.

> +
>  	if (retval) {
> -		ret_obj = output.pointer;
>  		if (!ret_obj)
>  			return -ENODATA;

-- 
 i.

[PATCH 2/3] platform/x86: lenovo-wmi-other: Balance IDA id allocation and free

[Rong Zhang]

Currently, the IDA id is only freed on wmi-other device removal or
failure to create firmware-attributes device, kset, or attributes. It
leaks IDA ids if the wmi-other device is bound multiple times, as the
unbind callback never frees the previously allocated IDA id.
Additionally, if the wmi-other device has failed to create a
firmware-attributes device before it gets removed, the wmi-device
removal callback double frees the same IDA id.

These bugs were found by sashiko.dev [1].

Fix them by moving ida_free() into lwmi_om_fw_attr_remove() so it is
balanced with ida_alloc() in lwmi_om_fw_attr_add(). With them fixed,
properly set and utilize the validity of priv->ida_id to balance
firmware-attributes registration and removal, without relying on
propagating the registration error to the component framework, which is
more reliable and aligns with the hwmon device registration and removal
sequences.

No functional change intended.

Fixes: edc4b183b794 ("platform/x86: Add Lenovo Other Mode WMI Driver")
Cc: stable@vger.kernel.org
Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-other.c | 34 +++++++++++++++----------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 6040f45aa2b0..b47418df099f 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -957,17 +957,17 @@ static struct capdata01_attr_group cd01_attr_groups[] = {
 /**
  * lwmi_om_fw_attr_add() - Register all firmware_attributes_class members
  * @priv: The Other Mode driver data.
- *
- * Return: Either 0, or an error code.
  */
-static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
+static void lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
 {
 	unsigned int i;
 	int err;
 
 	priv->ida_id = ida_alloc(&lwmi_om_ida, GFP_KERNEL);
-	if (priv->ida_id < 0)
-		return priv->ida_id;
+	if (priv->ida_id < 0) {
+		err = priv->ida_id;
+		goto err;
+	}
 
 	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
 					  MKDEV(0, 0), NULL, "%s-%u",
@@ -993,7 +993,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
 
 		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
 	}
-	return 0;
+	return;
 
 err_remove_groups:
 	while (i--)
@@ -1007,7 +1007,12 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
 
 err_free_ida:
 	ida_free(&lwmi_om_ida, priv->ida_id);
-	return err;
+
+err:
+	priv->ida_id = -EIDRM;
+
+	dev_warn(&priv->wdev->dev,
+		 "failed to register firmware-attributes device: %d\n", err);
 }
 
 /**
@@ -1016,12 +1021,17 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
  */
 static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
 {
+	if (priv->ida_id < 0)
+		return;
+
 	for (unsigned int i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++)
 		sysfs_remove_group(&priv->fw_attr_kset->kobj,
 				   cd01_attr_groups[i].attr_group);
 
 	kset_unregister(priv->fw_attr_kset);
 	device_unregister(priv->fw_attr_dev);
+	ida_free(&lwmi_om_ida, priv->ida_id);
+	priv->ida_id = -EIDRM;
 }
 
 /* ======== Self (master: lenovo-wmi-other) ======== */
@@ -1063,7 +1073,9 @@ static int lwmi_om_master_bind(struct device *dev)
 
 	lwmi_om_fan_info_collect_cd00(priv);
 
-	return lwmi_om_fw_attr_add(priv);
+	lwmi_om_fw_attr_add(priv);
+
+	return 0;
 }
 
 /**
@@ -1115,13 +1127,7 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
 
 static void lwmi_other_remove(struct wmi_device *wdev)
 {
-	struct lwmi_om_priv *priv = dev_get_drvdata(&wdev->dev);
-
 	component_master_del(&wdev->dev, &lwmi_om_master_ops);
-
-	/* No IDA to free if the driver is never bound to its components. */
-	if (priv->ida_id >= 0)
-		ida_free(&lwmi_om_ida, priv->ida_id);
 }
 
 static const struct wmi_device_id lwmi_other_id_table[] = {
-- 
2.53.0

Re: [PATCH 2/3] platform/x86: lenovo-wmi-other: Balance IDA id allocation and free

[Ilpo Järvinen]

On Thu, 2 Apr 2026, Rong Zhang wrote:

> Currently, the IDA id is only freed on wmi-other device removal or
> failure to create firmware-attributes device, kset, or attributes. It
> leaks IDA ids if the wmi-other device is bound multiple times, as the
> unbind callback never frees the previously allocated IDA id.
> Additionally, if the wmi-other device has failed to create a
> firmware-attributes device before it gets removed, the wmi-device
> removal callback double frees the same IDA id.
> 
> These bugs were found by sashiko.dev [1].
> 
> Fix them by moving ida_free() into lwmi_om_fw_attr_remove() so it is
> balanced with ida_alloc() in lwmi_om_fw_attr_add(). With them fixed,
> properly set and utilize the validity of priv->ida_id to balance
> firmware-attributes registration and removal, without relying on
> propagating the registration error to the component framework, which is
> more reliable and aligns with the hwmon device registration and removal
> sequences.
> 
> No functional change intended.
> 
> Fixes: edc4b183b794 ("platform/x86: Add Lenovo Other Mode WMI Driver")
> Cc: stable@vger.kernel.org
> Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 34 +++++++++++++++----------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 6040f45aa2b0..b47418df099f 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -957,17 +957,17 @@ static struct capdata01_attr_group cd01_attr_groups[] = {
>  /**
>   * lwmi_om_fw_attr_add() - Register all firmware_attributes_class members
>   * @priv: The Other Mode driver data.
> - *
> - * Return: Either 0, or an error code.
>   */
> -static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> +static void lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  {
>  	unsigned int i;
>  	int err;
>  
>  	priv->ida_id = ida_alloc(&lwmi_om_ida, GFP_KERNEL);
> -	if (priv->ida_id < 0)
> -		return priv->ida_id;
> +	if (priv->ida_id < 0) {
> +		err = priv->ida_id;

This looks a bit backwards. It would be better to do:

	err = ida_alloc(&lwmi_om_ida, GFP_KERNEL);
	if (err < 0)
> +		goto err;

	priv->ida_id = err;

...This, btw, tells us why "ret" would have been superior name for the 
generic return variable as it does not carry "error" connotation.

> +	}
>  
>  	priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
>  					  MKDEV(0, 0), NULL, "%s-%u",
> @@ -993,7 +993,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  
>  		cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
>  	}
> -	return 0;
> +	return;
>  
>  err_remove_groups:
>  	while (i--)
> @@ -1007,7 +1007,12 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>  
>  err_free_ida:
>  	ida_free(&lwmi_om_ida, priv->ida_id);
> -	return err;
> +
> +err:
> +	priv->ida_id = -EIDRM;
> +
> +	dev_warn(&priv->wdev->dev,
> +		 "failed to register firmware-attributes device: %d\n", err);
>  }
>  
>  /**
> @@ -1016,12 +1021,17 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
>   */
>  static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
>  {
> +	if (priv->ida_id < 0)
> +		return;
> +
>  	for (unsigned int i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++)
>  		sysfs_remove_group(&priv->fw_attr_kset->kobj,
>  				   cd01_attr_groups[i].attr_group);
>  
>  	kset_unregister(priv->fw_attr_kset);
>  	device_unregister(priv->fw_attr_dev);
> +	ida_free(&lwmi_om_ida, priv->ida_id);
> +	priv->ida_id = -EIDRM;
>  }
>  
>  /* ======== Self (master: lenovo-wmi-other) ======== */
> @@ -1063,7 +1073,9 @@ static int lwmi_om_master_bind(struct device *dev)
>  
>  	lwmi_om_fan_info_collect_cd00(priv);
>  
> -	return lwmi_om_fw_attr_add(priv);
> +	lwmi_om_fw_attr_add(priv);
> +
> +	return 0;
>  }
>  
>  /**
> @@ -1115,13 +1127,7 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
>  
>  static void lwmi_other_remove(struct wmi_device *wdev)
>  {
> -	struct lwmi_om_priv *priv = dev_get_drvdata(&wdev->dev);
> -
>  	component_master_del(&wdev->dev, &lwmi_om_master_ops);
> -
> -	/* No IDA to free if the driver is never bound to its components. */
> -	if (priv->ida_id >= 0)
> -		ida_free(&lwmi_om_ida, priv->ida_id);
>  }
>  
>  static const struct wmi_device_id lwmi_other_id_table[] = {
> 

-- 
 i.

Re: [PATCH 2/3] platform/x86: lenovo-wmi-other: Balance IDA id allocation and free

[Derek John Clark]

On Thu, Apr 9, 2026 at 5:30 AM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Thu, 2 Apr 2026, Rong Zhang wrote:
>
> > Currently, the IDA id is only freed on wmi-other device removal or
> > failure to create firmware-attributes device, kset, or attributes. It
> > leaks IDA ids if the wmi-other device is bound multiple times, as the
> > unbind callback never frees the previously allocated IDA id.
> > Additionally, if the wmi-other device has failed to create a
> > firmware-attributes device before it gets removed, the wmi-device
> > removal callback double frees the same IDA id.
> >
> > These bugs were found by sashiko.dev [1].
> >
> > Fix them by moving ida_free() into lwmi_om_fw_attr_remove() so it is
> > balanced with ida_alloc() in lwmi_om_fw_attr_add(). With them fixed,
> > properly set and utilize the validity of priv->ida_id to balance
> > firmware-attributes registration and removal, without relying on
> > propagating the registration error to the component framework, which is
> > more reliable and aligns with the hwmon device registration and removal
> > sequences.
> >
> > No functional change intended.
> >
> > Fixes: edc4b183b794 ("platform/x86: Add Lenovo Other Mode WMI Driver")
> > Cc: stable@vger.kernel.org
> > Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> >  drivers/platform/x86/lenovo/wmi-other.c | 34 +++++++++++++++----------
> >  1 file changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> > index 6040f45aa2b0..b47418df099f 100644
> > --- a/drivers/platform/x86/lenovo/wmi-other.c
> > +++ b/drivers/platform/x86/lenovo/wmi-other.c
> > @@ -957,17 +957,17 @@ static struct capdata01_attr_group cd01_attr_groups[] = {
> >  /**
> >   * lwmi_om_fw_attr_add() - Register all firmware_attributes_class members
> >   * @priv: The Other Mode driver data.
> > - *
> > - * Return: Either 0, or an error code.
> >   */
> > -static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> > +static void lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> >  {
> >       unsigned int i;
> >       int err;
> >
> >       priv->ida_id = ida_alloc(&lwmi_om_ida, GFP_KERNEL);
> > -     if (priv->ida_id < 0)
> > -             return priv->ida_id;
> > +     if (priv->ida_id < 0) {
> > +             err = priv->ida_id;
>
> This looks a bit backwards. It would be better to do:
>
>         err = ida_alloc(&lwmi_om_ida, GFP_KERNEL);
>         if (err < 0)
> > +             goto err;
>
>         priv->ida_id = err;
>
> ...This, btw, tells us why "ret" would have been superior name for the
> generic return variable as it does not carry "error" connotation.
>

I've taken a note of this. In the next series I'll add a patch to
rename any "err" to "ret" for all lenovo wmi drivers. This series is
already getting out of hand though so I don't want to introduce more
complexity at this time. For now I'll just invert the logic as stated.

Thanks,
Derek

> > +     }
> >
> >       priv->fw_attr_dev = device_create(&firmware_attributes_class, NULL,
> >                                         MKDEV(0, 0), NULL, "%s-%u",
> > @@ -993,7 +993,7 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> >
> >               cd01_attr_groups[i].tunable_attr->dev = &priv->wdev->dev;
> >       }
> > -     return 0;
> > +     return;
> >
> >  err_remove_groups:
> >       while (i--)
> > @@ -1007,7 +1007,12 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> >
> >  err_free_ida:
> >       ida_free(&lwmi_om_ida, priv->ida_id);
> > -     return err;
> > +
> > +err:
> > +     priv->ida_id = -EIDRM;
> > +
> > +     dev_warn(&priv->wdev->dev,
> > +              "failed to register firmware-attributes device: %d\n", err);
> >  }
> >
> >  /**
> > @@ -1016,12 +1021,17 @@ static int lwmi_om_fw_attr_add(struct lwmi_om_priv *priv)
> >   */
> >  static void lwmi_om_fw_attr_remove(struct lwmi_om_priv *priv)
> >  {
> > +     if (priv->ida_id < 0)
> > +             return;
> > +
> >       for (unsigned int i = 0; i < ARRAY_SIZE(cd01_attr_groups) - 1; i++)
> >               sysfs_remove_group(&priv->fw_attr_kset->kobj,
> >                                  cd01_attr_groups[i].attr_group);
> >
> >       kset_unregister(priv->fw_attr_kset);
> >       device_unregister(priv->fw_attr_dev);
> > +     ida_free(&lwmi_om_ida, priv->ida_id);
> > +     priv->ida_id = -EIDRM;
> >  }
> >
> >  /* ======== Self (master: lenovo-wmi-other) ======== */
> > @@ -1063,7 +1073,9 @@ static int lwmi_om_master_bind(struct device *dev)
> >
> >       lwmi_om_fan_info_collect_cd00(priv);
> >
> > -     return lwmi_om_fw_attr_add(priv);
> > +     lwmi_om_fw_attr_add(priv);
> > +
> > +     return 0;
> >  }
> >
> >  /**
> > @@ -1115,13 +1127,7 @@ static int lwmi_other_probe(struct wmi_device *wdev, const void *context)
> >
> >  static void lwmi_other_remove(struct wmi_device *wdev)
> >  {
> > -     struct lwmi_om_priv *priv = dev_get_drvdata(&wdev->dev);
> > -
> >       component_master_del(&wdev->dev, &lwmi_om_master_ops);
> > -
> > -     /* No IDA to free if the driver is never bound to its components. */
> > -     if (priv->ida_id >= 0)
> > -             ida_free(&lwmi_om_ida, priv->ida_id);
> >  }
> >
> >  static const struct wmi_device_id lwmi_other_id_table[] = {
> >
>
> --
>  i.
>

[PATCH 3/3] platform/x86: lenovo-wmi-other: Balance component bind and unbind

[Rong Zhang]

When lwmi_om_master_bind() fails, the master device's components are
left bound, with the aggregate device destroyed due to the failure
(found by sashiko.dev [1]).

Balance calls to component_bind_all() and component_unbind_all() when an
error is propagated to the component framework.

No functional change intended.

Fixes: edc4b183b794 ("platform/x86: Add Lenovo Other Mode WMI Driver")
Cc: stable@vger.kernel.org
Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/wmi-other.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index b47418df099f..4b47b5886e33 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -1068,8 +1068,11 @@ static int lwmi_om_master_bind(struct device *dev)
 
 	priv->cd00_list = binder.cd00_list;
 	priv->cd01_list = binder.cd01_list;
-	if (!priv->cd00_list || !priv->cd01_list)
+	if (!priv->cd00_list || !priv->cd01_list) {
+		component_unbind_all(dev, NULL);
+
 		return -ENODEV;
+	}
 
 	lwmi_om_fan_info_collect_cd00(priv);
 
-- 
2.53.0

Re: [PATCH 3/3] platform/x86: lenovo-wmi-other: Balance component bind and unbind

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

On Thu, 2 Apr 2026, Rong Zhang wrote:

> When lwmi_om_master_bind() fails, the master device's components are
> left bound, with the aggregate device destroyed due to the failure
> (found by sashiko.dev [1]).
> 
> Balance calls to component_bind_all() and component_unbind_all() when an
> error is propagated to the component framework.
> 
> No functional change intended.
> 
> Fixes: edc4b183b794 ("platform/x86: Add Lenovo Other Mode WMI Driver")
> Cc: stable@vger.kernel.org
> Link: https://sashiko.dev/#/patchset/20260331181208.421552-1-derekjohn.clark%40gmail.com [1]
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/wmi-other.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index b47418df099f..4b47b5886e33 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -1068,8 +1068,11 @@ static int lwmi_om_master_bind(struct device *dev)
>  
>  	priv->cd00_list = binder.cd00_list;
>  	priv->cd01_list = binder.cd01_list;
> -	if (!priv->cd00_list || !priv->cd01_list)
> +	if (!priv->cd00_list || !priv->cd01_list) {
> +		component_unbind_all(dev, NULL);
> +
>  		return -ENODEV;
> +	}
>  
>  	lwmi_om_fan_info_collect_cd00(priv);
>  
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.