[PATCH] hwmon: (hp-wmi-sensors) Fix use-after-free in fungible_show()

[PATCH] hwmon: (hp-wmi-sensors) Fix use-after-free in fungible_show()

[Muhammad Bilal]

nsensor->current_state is dynamically replaced as the sensor's state
changes. update_numeric_sensor_from_wobj() does this by freeing the
old string and installing a new one:

	if (strcmp(trimmed, nsensor->current_state)) {
		new_string = hp_wmi_strdup(dev, trimmed);
		if (new_string) {
			devm_kfree(dev, nsensor->current_state);
			nsensor->current_state = new_string;
		}
	}

This function is only ever called from hp_wmi_update_info() while
state->lock is held, so the free-and-replace itself is properly
serialized against concurrent updates.

fungible_show(), however, reads the same pointer after the lock has
already been dropped:

	err = hp_wmi_update_info(state, info);
	if (err)
		return err;

	switch (prop) {
	...
	case HP_WMI_PROPERTY_CURRENT_STATE:
		seq_printf(seqf, "%s\n", nsensor->current_state);
		break;

hp_wmi_update_info() takes state->lock internally and releases it
before returning, so by the time fungible_show() dereferences
nsensor->current_state in seq_printf(), no lock is held. Two
processes reading a sensor's current_state debugfs entry at
overlapping times (or one reading it while another read of the same
sensor triggers a refresh) can race: one thread's seq_printf() can
be part-way through printing the string at the moment another
thread's call into update_numeric_sensor_from_wobj() frees it with
devm_kfree() and installs a new pointer, causing a use-after-free
read.

Take state->lock around the read in fungible_show() as well, so it
can never run concurrently with the free-and-replace in
update_numeric_sensor_from_wobj().

Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/hwmon/hp-wmi-sensors.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index 03c684b..55aee16 100644
--- a/drivers/hwmon/hp-wmi-sensors.c
+++ b/drivers/hwmon/hp-wmi-sensors.c
@@ -1247,7 +1247,9 @@ static int fungible_show(struct seq_file *seqf, enum hp_wmi_property prop)
 		break;
 
 	case HP_WMI_PROPERTY_CURRENT_STATE:
+		mutex_lock(&state->lock);
 		seq_printf(seqf, "%s\n", nsensor->current_state);
+		mutex_unlock(&state->lock);
 		break;
 
 	case HP_WMI_PROPERTY_UNIT_MODIFIER:
-- 
2.43.0

Re: [PATCH] hwmon: (hp-wmi-sensors) Fix use-after-free in fungible_show()

[James Seo]

On Wed, Sep 16, 2026 at 05:29:26AM +0500, Muhammad Bilal wrote:
> hp_wmi_update_info() takes state->lock internally and releases it
> before returning, so by the time fungible_show() dereferences
> nsensor->current_state in seq_printf(), no lock is held. Two
> processes reading a sensor's current_state debugfs entry at
> overlapping times (or one reading it while another read of the same
> sensor triggers a refresh) can race: one thread's seq_printf() can
> be part-way through printing the string at the moment another
> thread's call into update_numeric_sensor_from_wobj() frees it with
> devm_kfree() and installs a new pointer, causing a use-after-free
> read.
Thanks for identifying this issue.

> Take state->lock around the read in fungible_show() as well, so it
> can never run concurrently with the free-and-replace in
> update_numeric_sensor_from_wobj().
> 
> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  drivers/hwmon/hp-wmi-sensors.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
> index 03c684b..55aee16 100644
> --- a/drivers/hwmon/hp-wmi-sensors.c
> +++ b/drivers/hwmon/hp-wmi-sensors.c
> @@ -1247,7 +1247,9 @@ static int fungible_show(struct seq_file *seqf, enum hp_wmi_property prop)
>  		break;
>  
>  	case HP_WMI_PROPERTY_CURRENT_STATE:
> +		mutex_lock(&state->lock);
>  		seq_printf(seqf, "%s\n", nsensor->current_state);
> +		mutex_unlock(&state->lock);
>  		break;
>  
>  	case HP_WMI_PROPERTY_UNIT_MODIFIER:
> -- 
> 2.43.0
>
Fine with me.

Acked-by: James Seo <james@equiv.tech>

Re: [PATCH] hwmon: (hp-wmi-sensors) Fix use-after-free in fungible_show()

[Guenter Roeck]

On Wed, Sep 16, 2026 at 05:29:26AM +0500, Muhammad Bilal wrote:
> nsensor->current_state is dynamically replaced as the sensor's state
> changes. update_numeric_sensor_from_wobj() does this by freeing the
> old string and installing a new one:
> 
> 	if (strcmp(trimmed, nsensor->current_state)) {
> 		new_string = hp_wmi_strdup(dev, trimmed);
> 		if (new_string) {
> 			devm_kfree(dev, nsensor->current_state);
> 			nsensor->current_state = new_string;
> 		}
> 	}
> 
> This function is only ever called from hp_wmi_update_info() while
> state->lock is held, so the free-and-replace itself is properly
> serialized against concurrent updates.
> 
> fungible_show(), however, reads the same pointer after the lock has
> already been dropped:
> 
> 	err = hp_wmi_update_info(state, info);
> 	if (err)
> 		return err;
> 
> 	switch (prop) {
> 	...
> 	case HP_WMI_PROPERTY_CURRENT_STATE:
> 		seq_printf(seqf, "%s\n", nsensor->current_state);
> 		break;
> 
> hp_wmi_update_info() takes state->lock internally and releases it
> before returning, so by the time fungible_show() dereferences
> nsensor->current_state in seq_printf(), no lock is held. Two
> processes reading a sensor's current_state debugfs entry at
> overlapping times (or one reading it while another read of the same
> sensor triggers a refresh) can race: one thread's seq_printf() can
> be part-way through printing the string at the moment another
> thread's call into update_numeric_sensor_from_wobj() frees it with
> devm_kfree() and installs a new pointer, causing a use-after-free
> read.
> 
> Take state->lock around the read in fungible_show() as well, so it
> can never run concurrently with the free-and-replace in
> update_numeric_sensor_from_wobj().
> 
> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> Acked-by: James Seo <james@equiv.tech>

Applied.

Thanks,
Guenter