[PATCH 2/2] PM: cpu: Restore synchronize_rcu() to cpu_pm_unregister_notifier()
Bradley Morgan <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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]>
---
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