The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/2] PM: cpu: Restore the RCU grace period to cpu_pm_unregister_notifier()
@ 2026-08-13  7:43 Bradley Morgan
  2026-08-13  7:43 ` [PATCH 1/2] clocksource/drivers/timer-ti-dm: Unregister CPU PM notifier outside of the timer lock Bradley Morgan
  2026-08-13  7:43 ` [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier() Bradley Morgan
  0 siblings, 2 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-08-13  7:43 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Thierry Reding, Daniel Leznan, Thomas Gleixner,
	Valentin Schneider, Rosen Penev, linux-pm, linux-kernel

Since commit b2f6662ac08d ("PM: cpu: Make notifier chain use a
raw_spinlock_t") cpu_pm_unregister_notifier() no longer waits for an
RCU grace period, but cpu_pm_notify() still walks the chain with only
rcu_read_lock() held. A driver that frees its notifier block right
after unregistering can race with a concurrent walker.

Patch 1 moves the one caller that unregisters from atomic context out
from under its spinlock, patch 2 restores the grace period.

Bradley Morgan (2):
  clocksource/drivers/timer-ti-dm: Unregister CPU PM notifier outside of
    the timer lock
  PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()

 drivers/clocksource/timer-ti-dm.c | 9 ++++++---
 kernel/cpu_pm.c                   | 9 ++++++++-
 2 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] clocksource/drivers/timer-ti-dm: Unregister CPU PM notifier outside of the timer lock
  2026-08-13  7:43 [PATCH 0/2] PM: cpu: Restore the RCU grace period to cpu_pm_unregister_notifier() Bradley Morgan
@ 2026-08-13  7:43 ` Bradley Morgan
  2026-08-13  7:43 ` [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier() Bradley Morgan
  1 sibling, 0 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-08-13  7:43 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Thierry Reding, Daniel Leznan, Thomas Gleixner,
	Valentin Schneider, Rosen Penev, linux-pm, linux-kernel

omap_dm_timer_remove() calls cpu_pm_unregister_notifier() with
dm_timer_lock held and interrupts disabled. Nothing sleeps in there
today, but it pins the helper into a context where it can never be
allowed to sleep, which is in the way of restoring the RCU grace
period on the cpu_pm notifier chain.

Do the list lookup under the lock and move the unregister after the
unlock. Nothing can race it at that point: remove() owns the device,
and once the timer is off the list nobody can reach it anymore.

Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 drivers/clocksource/timer-ti-dm.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index bd06afb7d522..032e102bdd0a 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -1530,7 +1530,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
  */
 static void omap_dm_timer_remove(struct platform_device *pdev)
 {
-	struct dmtimer *timer;
+	struct dmtimer *timer, *found = NULL;
 	unsigned long flags;
 	int ret = -EINVAL;
 
@@ -1538,14 +1538,17 @@ static void omap_dm_timer_remove(struct platform_device *pdev)
 	list_for_each_entry(timer, &omap_timer_list, node)
 		if (!strcmp(dev_name(&timer->pdev->dev),
 			    dev_name(&pdev->dev))) {
-			if (!(timer->capability & OMAP_TIMER_ALWON))
-				cpu_pm_unregister_notifier(&timer->nb);
 			list_del(&timer->node);
+			found = timer;
 			ret = 0;
 			break;
 		}
 	spin_unlock_irqrestore(&dm_timer_lock, flags);
 
+	/* Unregister outside the lock: cpu_pm_unregister_notifier() may sleep. */
+	if (found && !(found->capability & OMAP_TIMER_ALWON))
+		cpu_pm_unregister_notifier(&found->nb);
+
 	pm_runtime_disable(&pdev->dev);
 
 	if (ret)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()
  2026-08-13  7:43 [PATCH 0/2] PM: cpu: Restore the RCU grace period to cpu_pm_unregister_notifier() Bradley Morgan
  2026-08-13  7:43 ` [PATCH 1/2] clocksource/drivers/timer-ti-dm: Unregister CPU PM notifier outside of the timer lock Bradley Morgan
@ 2026-08-13  7:43 ` Bradley Morgan
  2026-08-20 21:24   ` Rafael J. Wysocki (Intel)
  1 sibling, 1 reply; 5+ messages in thread
From: Bradley Morgan @ 2026-08-13  7:43 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Thierry Reding, Daniel Leznan, Thomas Gleixner,
	Valentin Schneider, Rosen Penev, linux-pm, linux-kernel

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@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()
  2026-08-13  7:43 ` [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier() Bradley Morgan
@ 2026-08-20 21:24   ` Rafael J. Wysocki (Intel)
  2026-08-20 21:28     ` Bradley Morgan
  0 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-08-20 21:24 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: Rafael J . Wysocki, Thierry Reding, Daniel Leznan,
	Thomas Gleixner, Valentin Schneider, Rosen Penev, linux-pm,
	linux-kernel

On Thu, Aug 13, 2026 at 9:43 AM Bradley Morgan <include@grrlz.net> 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@vger.kernel.org
> Signed-off-by: Bradley Morgan <include@grrlz.net>

Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()
  2026-08-20 21:24   ` Rafael J. Wysocki (Intel)
@ 2026-08-20 21:28     ` Bradley Morgan
  0 siblings, 0 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-08-20 21:28 UTC (permalink / raw)
  To: Rafael J. Wysocki (Intel)
  Cc: Rafael J . Wysocki, Thierry Reding, Daniel Leznan,
	Thomas Gleixner, Valentin Schneider, Rosen Penev, linux-pm,
	linux-kernel

On 20 August 2026 22:24:51 BST, "Rafael J. Wysocki (Intel)"
<rafael@kernel.org> wrote:
>On Thu, Aug 13, 2026 at 9:43 AM Bradley Morgan <include@grrlz.net> 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@vger.kernel.org
>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>
>Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
>
>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!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-20 21:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  7:43 [PATCH 0/2] PM: cpu: Restore the RCU grace period to cpu_pm_unregister_notifier() Bradley Morgan
2026-08-13  7:43 ` [PATCH 1/2] clocksource/drivers/timer-ti-dm: Unregister CPU PM notifier outside of the timer lock Bradley Morgan
2026-08-13  7:43 ` [PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier() Bradley Morgan
2026-08-20 21:24   ` Rafael J. Wysocki (Intel)
2026-08-20 21:28     ` Bradley Morgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox