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

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

[Cong Nguyen]

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);
+}
+
 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);
-- 
2.25.1

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

[Guenter Roeck]

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);

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

[Nguyễn Công]

On Mon, Sep 14, 2026 at 10:40 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> 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

Makes sense, sent as v2 -- disable_work_sync() instead.

thanks,
Cong

> > +}
> > +
> >   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);
>