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

[Bradley Morgan]

On 20 August 2026 22:24:51 BST, "Rafael J. Wysocki (Intel)"
<rafael@xxxxxxxxxx> wrote:
>On Thu, Aug 13, 2026 at 9:43 AM Bradley Morgan <include@xxxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Bradley Morgan <include@xxxxxxxxx>
>
>Acked-by: Rafael J. Wysocki (Intel) <rafael@xxxxxxxxxx>
>
>or if you want me to pick up this series, I need an ACK on the first
>patch.
>

Waiting game... (And I can't take it! I'm not maintainer of anything
sadly, one day, one day, I'm manifesting it.. one day)

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

Thanks!