Re: [PATCH] hwmon: (gpio-fan) fix use-after-free of alarm_work on unbind

[Guenter Roeck <linux@roeck-us.net>]

On 9/14/26 04:54, Cong Nguyen wrote:
> fan_alarm_irq_handler() schedules alarm_work, but nothing ever cancels
> it. free_irq() (via devm) only waits for an in-progress IRQ handler,
> not queued work -- a pending alarm_work can run after fan_data is
> devm-freed, dereferencing it in fan_alarm_notify().
> 
> Cancel it via a devm action registered before devm_request_irq(), so
> teardown frees the IRQ first, then cancels whatever's already queued.
> 
> Fixes: d6fe1360f42e ("hwmon: add generic GPIO fan driver")
> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/r/20260830152150.27F5F1F000E9@smtp.kernel.org
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4
> Signed-off-by: Cong Nguyen <congnt264@gmail.com>
> ---
>   drivers/hwmon/gpio-fan.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 084828e1e281..b1b57bb63a33 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -81,9 +81,16 @@ static ssize_t fan1_alarm_show(struct device *dev,
>   
>   static DEVICE_ATTR_RO(fan1_alarm);
>   
> +static void gpio_fan_cancel_alarm_work(void *data)
> +{
> +	struct gpio_fan_data *fan_data = data;
> +
> +	cancel_work_sync(&fan_data->alarm_work);

I think this still leaves a window on teardown, if an interrupt happens after
the work was canceled. I would suggest to use disable_work_sync() instead.

Thanks,
Guenter
> +}
> +
>   static int fan_alarm_init(struct gpio_fan_data *fan_data)
>   {
> -	int alarm_irq;
> +	int alarm_irq, err;
>   	struct device *dev = fan_data->dev;
>   
>   	/*
> @@ -95,6 +102,17 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data)
>   		return 0;
>   
>   	INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
> +
> +	/*
> +	 * Register before devm_request_irq() below: LIFO teardown must free
> +	 * the IRQ (stopping new schedule_work() calls) before this cancels
> +	 * whatever alarm_work is already queued or running.
> +	 */
> +	err = devm_add_action_or_reset(dev, gpio_fan_cancel_alarm_work,
> +					fan_data);
> +	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);