Re: [PATCH] platform: cznic: turris-omnia-mcu: stop button emulation producers

[Myeonghun Pak]

Thanks for the Ack.

Agreed that an autodisable helper would be useful. I’ll look into a
separate follow-up, including the impact on existing callers.

2026년 10월 19일 (월) 오전 5:48, Marek Behún <kabel@xxxxxxxxxx>님이 작성:
>
> Acked-by: Marek Behún <kabel@xxxxxxxxxx>
>
> This makes sense.
>
> But I wonder whether the devm_delayed_work_drop() from devm-helpers.h
> should not be changed to use disable_delayed_work_sync() instead of
> cancel_delayed_work_sync().
>
> In this case the initialization function in devm-helpers should be called
> devm_delayed_work_autodisable() instead of devm_delayed_work_autocancel(),
> though.
>
> On Thu, Oct 15, 2026 at 09:39:18PM -0400, Myeonghun Pak wrote:
> > The legacy MCU IRQ thread can requeue button-release emulation work
> > after its managed cleanup has canceled it. Since the IRQ is still active
> > at that point, the newly queued work can outlive device teardown and
> > access freed MCU state.
> >
> > Use disable_delayed_work_sync() to prevent further queueing and drain
> > the worker before the IRQ action is released. Keep the parent IRQ active
> > while the worker completes. devm_add_action_or_reset() also drains the
> > work if action registration fails.
> >
> > This issue was identified as part of our ongoing research while
> > reviewing kernel code.
> >
> > Fixes: dfa556e45ae9 ("platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs")
> > Assisted-by: LLM
> > Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> > Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> > Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> > ---
> >  .../platform/cznic/turris-omnia-mcu-gpio.c    | 31 +++++++++----------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/platform/cznic/turris-omnia-mcu-gpio.c b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
> > index 4e430d6c3fc4..8f43bfa92373 100644
> > --- a/drivers/platform/cznic/turris-omnia-mcu-gpio.c
> > +++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
> > @@ -11,7 +11,6 @@
> >  #include <linux/bug.h>
> >  #include <linux/cleanup.h>
> >  #include <linux/device.h>
> > -#include <linux/devm-helpers.h>
> >  #include <linux/errno.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/gpio/driver.h>
> > @@ -841,6 +840,18 @@ static void button_release_emul_fn(struct work_struct *work)
> >       generic_handle_irq_safe(mcu->client->irq);
> >  }
> >
> > +static void omnia_disable_button_release_work(void *data)
> > +{
> > +     struct delayed_work *work = data;
> > +
> > +     /*
> > +      * The IRQ thread may requeue this work while the IRQ remains active.
> > +      * Disable and drain it before releasing the IRQ action and the MCU
> > +      * state used by the worker.
> > +      */
> > +     disable_delayed_work_sync(work);
> > +}
> > +
> >  static void
> >  fill_int_from_sts(unsigned long *rising, unsigned long *falling,
> >                 unsigned long rising_sts, unsigned long falling_sts,
> > @@ -1082,21 +1093,9 @@ int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu)
> >               return dev_err_probe(dev, err, "Cannot request IRQ\n");
> >
> >       if (!new_api) {
> > -             /*
> > -              * The button_release_emul_work has to be initialized before the
> > -              * thread is requested, and on driver remove it needs to be
> > -              * canceled before the thread is freed. Therefore we can't use
> > -              * devm_delayed_work_autocancel() directly, because the order
> > -              *   devm_delayed_work_autocancel();
> > -              *   devm_request_threaded_irq();
> > -              * would cause improper release order:
> > -              *   free_irq();
> > -              *   cancel_delayed_work_sync();
> > -              * Instead we first initialize the work above, and only now
> > -              * after IRQ is requested we add the work devm action.
> > -              */
> > -             err = devm_add_action(dev, devm_delayed_work_drop,
> > -                                   &mcu->button_release_emul_work);
> > +             err = devm_add_action_or_reset(dev,
> > +                                            omnia_disable_button_release_work,
> > +                                            &mcu->button_release_emul_work);
> >               if (err)
> >                       return err;
> >       }