Re: [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()

"Rafael J. Wysocki (Intel)" <[email protected]>
Newsgroups org.kernel.vger.linux-pm,org.kernel.vger.linux-kernel
Message-ID <CAJZ5v0jJDUZ6HH44GK6sGdVKNAY+i7EMsqdFPNcGvoW4d7W4kg@mail.gmail.com>
On Thu, Aug 13, 2026 at 9:43 AM Bradley Morgan <[email protected]> wrote:
>
> cpu_pm_notify() walks the notifier chain lockless under only
> rcu_read_lock(). That only works if a removed notifier block is not
> freed until every concurrent walker is done with it.
>
> The chain used to be an atomic_notifier, whose unregister ends in
> synchronize_rcu(). Commit b2f6662ac08d ("PM: cpu: Make notifier chain
> use a raw_spinlock_t") switched it over to a raw_notifier, and in
> doing so replaced that with raw_notifier_chain_unregister(), which
> does not synchronize. The grace period quietly went away, so a driver
> that frees the memory holding its notifier block right after
> cpu_pm_unregister_notifier() returns can race with a concurrent
> walker:
>
>   cpu1 (idle exit)                  cpu2 (driver remove)
>   ----------------                  --------------------
>   cpu_pm_notify(CPU_PM_EXIT)
>     rcu_read_lock()
>     nb = rcu_dereference_raw(*nl)
>                                     cpu_pm_unregister_notifier(&od->nb)
>                                       unlink, no grace period
>                                       remove() frees od (devm)
>     nb->notifier_call(nb, ...)
>       /* use after free */
>
> The rcu_read_lock() on cpu1 does not stop cpu2 from freeing the
> block, and nothing else does. The read side is the idle exit path,
> so this can hit on any system where a driver with a cpu_pm notifier
> gets unbound.
>
> Add the grace period back. It is only needed when a notifier was
> actually removed, so wait on success and return -ENOENT without
> waiting otherwise. The kerneldoc gets back the "may sleep" note that
> the same commit dropped.
>
> Fixes: b2f6662ac08d ("PM: cpu: Make notifier chain use a raw_spinlock_t")
> Cc: [email protected]
> Signed-off-by: Bradley Morgan <[email protected]>

Acked-by: Rafael J. Wysocki (Intel) <[email protected]>

or if you want me to pick up this series, I need an ACK on the first patch.

> ---
>  kernel/cpu_pm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c
> index 7481fbb947d3..a2a598ad6e6d 100644
> --- a/kernel/cpu_pm.c
> +++ b/kernel/cpu_pm.c
> @@ -10,6 +10,7 @@
>  #include <linux/cpu_pm.h>
>  #include <linux/module.h>
>  #include <linux/notifier.h>
> +#include <linux/rcupdate.h>
>  #include <linux/spinlock.h>
>  #include <linux/syscore_ops.h>
>
> @@ -76,7 +77,8 @@ EXPORT_SYMBOL_GPL(cpu_pm_register_notifier);
>   *
>   * Remove a driver from the CPU PM notifier list.
>   *
> - * This function has the same return conditions as raw_notifier_chain_unregister.
> + * This function may sleep, and has the same return conditions as
> + * raw_notifier_chain_unregister.
>   */
>  int cpu_pm_unregister_notifier(struct notifier_block *nb)
>  {
> @@ -86,6 +88,11 @@ int cpu_pm_unregister_notifier(struct notifier_block *nb)
>         raw_spin_lock_irqsave(&cpu_pm_notifier.lock, flags);
>         ret = raw_notifier_chain_unregister(&cpu_pm_notifier.chain, nb);
>         raw_spin_unlock_irqrestore(&cpu_pm_notifier.lock, flags);
> +
> +       /* Wait for the rcu_read_lock() walkers in cpu_pm_notify(). */
> +       if (!ret)
> +               synchronize_rcu();
> +
>         return ret;
>  }
>  EXPORT_SYMBOL_GPL(cpu_pm_unregister_notifier);
> --
> 2.47.3
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.