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@xxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Link: https://lore.kernel.org/r/20260830152150.27F5F1F000E9@xxxxxxxxxxxxxxx
> > Cc: stable@xxxxxxxxxxxxxxx
> > Assisted-by: Claude:claude-opus-4
> > Signed-off-by: Cong Nguyen <congnt264@xxxxxxxxx>
> > ---
> >   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);
>