All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work
@ 2026-08-19  3:33 Fan Wu
  2026-08-19  3:43 ` sashiko-bot
  2026-08-19 20:35 ` Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Fan Wu @ 2026-08-19  3:33 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux, linux-kernel, stable, Fan Wu

fan_alarm_irq_handler() queues fan_data->alarm_work, but nothing
cancels it.  fan_alarm_notify() dereferences fan_data and its hwmon
device.  On unbind, devres frees the interrupt, which only waits for
the handler itself, and then releases the hwmon device and fan_data,
so a pending fan_alarm_notify() can run after those frees.

Replace INIT_WORK() with devm_work_autocancel(), registered before
devm_request_irq().  The devres cleanup then frees the interrupt
first, so no new work can be queued, and cancels the work while
fan_data and the hwmon device are still alive.

This issue was found by an in-house static analysis tool.

Fixes: d6fe1360f42e ("hwmon: add generic GPIO fan driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/hwmon/gpio-fan.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 084828e1e281..7f36e5f6f223 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/devm-helpers.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/kstrtox.h>
@@ -84,6 +85,7 @@ static DEVICE_ATTR_RO(fan1_alarm);
 static int fan_alarm_init(struct gpio_fan_data *fan_data)
 {
 	int alarm_irq;
+	int err;
 	struct device *dev = fan_data->dev;
 
 	/*
@@ -94,7 +96,11 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data)
 	if (alarm_irq <= 0)
 		return 0;
 
-	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
+	err = devm_work_autocancel(dev, &fan_data->alarm_work,
+				   fan_alarm_notify);
+	if (err)
+		return err;
+
 	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
 	return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
 				IRQF_SHARED, "GPIO fan alarm", fan_data);


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work
  2026-08-19  3:33 [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work Fan Wu
@ 2026-08-19  3:43 ` sashiko-bot
  2026-08-19 20:35 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-19  3:43 UTC (permalink / raw)
  To: Fan Wu; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The shared interrupt handler `fan_alarm_irq_handler` unconditionally returns `IRQ_NONE` despite successfully handling the interrupt by scheduling work.
- [High] The driver's devres management of `pm_runtime_disable` is unbalanced, leading to PM state leaks on unbind without control GPIOs, or corrupted PM state across probe failures.
- [High] Error paths in `set_fan_speed` and `gpio_fan_probe` cause software state to desynchronize from runtime PM state, triggering repeated usage count underflow warnings.
--

--- Patch [1]: [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work ---
commit 3d50f01d30cc7a7c52ecf0ad47ea90b3b37fc216
Author: Fan Wu <fanwu01@zju.edu.cn>

hwmon: (gpio-fan) fix use-after-free in alarm work

fan_alarm_irq_handler() queues fan_data->alarm_work, but nothing cancels it.
Replace INIT_WORK() with devm_work_autocancel() to safely cancel the work
during devres cleanup.

> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 084828e1e2817..7f36e5f6f2230 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -84,6 +85,7 @@ static DEVICE_ATTR_RO(fan1_alarm);
>  static int fan_alarm_init(struct gpio_fan_data *fan_data)
>  {
>  	int alarm_irq;
> +	int err;
>  	struct device *dev = fan_data->dev;
>  
>  	/*
> @@ -94,7 +96,11 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data)
>  	if (alarm_irq <= 0)
>  		return 0;
>  
> -	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
> +	err = devm_work_autocancel(dev, &fan_data->alarm_work,
> +				   fan_alarm_notify);
> +	if (err)
> +		return err;

[Severity: High]
This is a pre-existing issue, but does returning an error here expose an
unbalanced PM runtime state?

If fan_alarm_init() fails here, probe aborts and devres cleanup will execute
gpio_fan_stop(). Looking at gpio_fan_stop(), it unconditionally calls
pm_runtime_disable() on fan_data->dev even if pm_runtime_enable() was never
reached in probe.

Since gpio_fan_probe() registers gpio_fan_stop() via devm_add_action_or_reset()
before calling pm_runtime_enable(), couldn't this failure cause
pm_runtime_disable() to be called without a prior enable? This would increment
the disable depth to 2, permanently disabling runtime PM on subsequent deferred
probes.

Additionally, if the device has no control GPIOs, gpio_fan_stop() is never
registered. probe() unconditionally calls pm_runtime_enable(), but on unbind
pm_runtime_disable() is never called, leaking the PM state.

[Severity: High]
This is also a pre-existing issue, but can error paths in set_fan_speed() and
gpio_fan_probe() cause runtime PM usage count underflows?

In set_fan_speed(), if pm_runtime_put_sync() fails, it returns early:

drivers/hwmon/gpio-fan.c:set_fan_speed() {
    ...
    if (ret < 0 && ret != -ENOSYS)
        return ret;
    ...
}

This skips updating fan_data->speed_index = 0. The usage count was decremented,
but the software state assumes the fan is still active. Could subsequent calls
to stop the fan redundantly call pm_runtime_put_sync(), causing an underflow?

Similarly, in gpio_fan_probe(), if the fan is initially active (speed_index > 0)
but pm_runtime_resume_and_get(&pdev->dev) fails, probe returns an error without
clearing speed_index. During devres unwind, gpio_fan_stop() calls
set_fan_speed(0). Because speed_index > 0, it calls pm_runtime_put_sync() on a
usage count of 0, triggering an underflow warning.

> +
>  	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
>  	return devm_request_irq(dev, alarm_irq, fan_alarm_irq_handler,
>  				IRQF_SHARED, "GPIO fan alarm", fan_data);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does fan_alarm_irq_handler()
correctly return its IRQ status?

Looking at fan_alarm_irq_handler() in drivers/hwmon/gpio-fan.c:

drivers/hwmon/gpio-fan.c:fan_alarm_irq_handler() {
    ...
    schedule_work(&fan_data->alarm_work);

    return IRQ_NONE;
}

Since the handler unconditionally returns IRQ_NONE despite successfully
queueing the work, and the interrupt is registered as IRQF_SHARED, could the
kernel's spurious interrupt detector permanently disable the shared interrupt
line once the unhandled count exceeds the threshold?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819033317.446191-1-fanwu01@zju.edu.cn?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work
  2026-08-19  3:33 [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work Fan Wu
  2026-08-19  3:43 ` sashiko-bot
@ 2026-08-19 20:35 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2026-08-19 20:35 UTC (permalink / raw)
  To: Fan Wu; +Cc: linux-hwmon, linux-kernel, stable

On Wed, Aug 19, 2026 at 03:33:17AM +0000, Fan Wu wrote:
> fan_alarm_irq_handler() queues fan_data->alarm_work, but nothing
> cancels it.  fan_alarm_notify() dereferences fan_data and its hwmon
> device.  On unbind, devres frees the interrupt, which only waits for
> the handler itself, and then releases the hwmon device and fan_data,
> so a pending fan_alarm_notify() can run after those frees.
> 
> Replace INIT_WORK() with devm_work_autocancel(), registered before
> devm_request_irq().  The devres cleanup then frees the interrupt
> first, so no new work can be queued, and cancels the work while
> fan_data and the hwmon device are still alive.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: d6fe1360f42e ("hwmon: add generic GPIO fan driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Applied.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-19 20:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19  3:33 [PATCH] hwmon: (gpio-fan) fix use-after-free in alarm work Fan Wu
2026-08-19  3:43 ` sashiko-bot
2026-08-19 20:35 ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.