[PATCH v1] hwmon: (pwm-fan) Stop RPM timer before freeing tach data

[PATCH v1] hwmon: (pwm-fan) Stop RPM timer before freeing tach data

[Yibo Tan]

sample_timer() rearms the RPM timer and accesses the devm-managed
ctx->tachs and ctx->pulses_per_revolution arrays. The cleanup action
which stops the timer is registered before those arrays are allocated.

Since devres releases entries in reverse order, driver detach can free
the arrays before pwm_fan_cleanup() shuts down the timer. A timer expiry
in that window accesses the freed tach data.

With a KASAN kernel, a test-only kprobe delayed entry to
pwm_fan_cleanup() while normal sysfs unbind ran. Each of three runs
reported three four-byte reads and two four-byte writes in sample_timer()
after its backing devm allocations had been freed. The helper did not
invoke the timer callback, cleanup actions or free functions.

With the fix, three matching unbind runs completed without KASAN, BUG,
WARNING, Oops or panic. Instrumentation confirmed that timer retirement
completed before the first timer backing allocation was released.

Split timer retirement from the power cleanup and register its devres
action after the timer backing data and IRQ actions are installed. This
preserves the early power rollback action while ensuring the timer is
retired before its backing data is released. Use timer_shutdown_sync()
because the callback can rearm itself.

Fixes: 01695410d452 ("hwmon: (pwm-fan) Store tach data separately")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
---
 drivers/hwmon/pwm-fan.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 3b87f65bae05..c633d7f6464c 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -483,7 +483,6 @@ static void pwm_fan_cleanup(void *__ctx)
 {
 	struct pwm_fan_ctx *ctx = __ctx;
 
-	timer_delete_sync(&ctx->rpm_timer);
 	if (ctx->pwm_shutdown) {
 		ctx->enable_mode = pwm_enable_reg_enable;
 		__set_pwm(ctx, ctx->pwm_shutdown);
@@ -494,6 +493,13 @@ static void pwm_fan_cleanup(void *__ctx)
 	}
 }
 
+static void pwm_fan_timer_cleanup(void *__ctx)
+{
+	struct pwm_fan_ctx *ctx = __ctx;
+
+	timer_shutdown_sync(&ctx->rpm_timer);
+}
+
 static int pwm_fan_probe(struct platform_device *pdev)
 {
 	struct thermal_cooling_device *cdev;
@@ -644,6 +650,10 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	}
 
 	if (ctx->tach_count > 0) {
+		ret = devm_add_action_or_reset(dev, pwm_fan_timer_cleanup, ctx);
+		if (ret)
+			return ret;
+
 		ctx->sample_start = ktime_get();
 		mod_timer(&ctx->rpm_timer, jiffies + HZ);
 
@@ -700,6 +710,7 @@ static void pwm_fan_shutdown(struct platform_device *pdev)
 {
 	struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
 
+	pwm_fan_timer_cleanup(ctx);
 	pwm_fan_cleanup(ctx);
 }
 
-- 
2.39.5

Re: [PATCH v1] hwmon: (pwm-fan) Stop RPM timer before freeing tach data

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing lock in pwm_fan_cleanup leads to data race and potential memory/HW state corruption during system shutdown.
--

commit 7dfed1a79b0f4d06cbff416478ab737ce3920c29
Author: Yibo Tan <lhfff@tju.edu.cn>

hwmon: (pwm-fan) Stop RPM timer before freeing tach data

This commit fixes a use-after-free bug where driver detach can free timer
backing arrays before the timer is shut down. It does this by splitting the
timer retirement from power cleanup and registering its devres action after
the timer backing data is installed.

> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 3b87f65bae058..c633d7f6464c7 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c

[ ... ]

> @@ -700,6 +710,7 @@ static void pwm_fan_shutdown(struct platform_device *pdev)
>  {
>  	struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
>  
> +	pwm_fan_timer_cleanup(ctx);
>  	pwm_fan_cleanup(ctx);

[Severity: High]
This is a pre-existing issue, but does this unlocked call to
pwm_fan_cleanup() create a data race with the thermal subsystem?

During system shutdown, pwm_fan_shutdown() invokes pwm_fan_cleanup()
without holding ctx->lock. At the same time, if the kernel's thermal
governor triggers a temperature update, it can follow this path:

pwm_fan_set_cur_state()
  set_pwm()
    __set_pwm()

The thermal path acquires ctx->lock, but because pwm_fan_cleanup() modifies
ctx->enable_mode and ctx->pwm_state locklessly, these operations can race.
Could this result in invalid configurations being passed to the PWM driver
during shutdown, potentially causing a crash or malfunction?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911071809.130151-1-lhfff@tju.edu.cn?part=1

Re: [PATCH v1] hwmon: (pwm-fan) Stop RPM timer before freeing tach data

[Guenter Roeck]

On Fri, Sep 11, 2026 at 03:18:09PM +0800, Yibo Tan wrote:
> sample_timer() rearms the RPM timer and accesses the devm-managed
> ctx->tachs and ctx->pulses_per_revolution arrays. The cleanup action
> which stops the timer is registered before those arrays are allocated.
> 
> Since devres releases entries in reverse order, driver detach can free
> the arrays before pwm_fan_cleanup() shuts down the timer. A timer expiry
> in that window accesses the freed tach data.
> 
> With a KASAN kernel, a test-only kprobe delayed entry to
> pwm_fan_cleanup() while normal sysfs unbind ran. Each of three runs
> reported three four-byte reads and two four-byte writes in sample_timer()
> after its backing devm allocations had been freed. The helper did not
> invoke the timer callback, cleanup actions or free functions.
> 
> With the fix, three matching unbind runs completed without KASAN, BUG,
> WARNING, Oops or panic. Instrumentation confirmed that timer retirement
> completed before the first timer backing allocation was released.
> 
> Split timer retirement from the power cleanup and register its devres
> action after the timer backing data and IRQ actions are installed. This
> preserves the early power rollback action while ensuring the timer is
> retired before its backing data is released. Use timer_shutdown_sync()
> because the callback can rearm itself.
> 
> Fixes: 01695410d452 ("hwmon: (pwm-fan) Store tach data separately")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:GPT-5
> Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>

Applied.

Thanks,
Guenter