]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
power: sequencing: fix ABBA deadlock in pwrseq_device_unregister()
authorBartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Thu, 18 Jun 2026 08:45:12 +0000 (10:45 +0200)
committerBartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Mon, 22 Jun 2026 08:05:39 +0000 (10:05 +0200)
The pwrseq core takes three locks in consistent order everywhere:

  pwrseq_sem -> pwrseq->rw_lock -> pwrseq->state_lock

pwrseq_get() -> pwrseq_match_device() takes pwrseq_sem for reading, then
rw_lock for reading. pwrseq_power_on()/pwrseq_power_off() take rw_lock
for reading and then state_lock.

pwrseq_device_unregister() is the only exception, it takes: state_lock,
then rw_lock for writing and finally pwrseq_sem for writing. This created
two potential ABBA deadlock situations that sashiko pointed out.

  - pwrseq_power_on/off() take rw_lock for reading then state_lock, while
    pwrseq_unregister() takes state_lock then rw_lock for writing
  - pwrseq_get() takes pwrseq_sem for reading then rw_lock for reading,
    while pwrseq_unregister() takes rw_lock for writing then pwrseq_sem
    for writing

Reorder the unregister path to taking pwrseq_sem for writing -> rw_lock
for writing and drop the state_lock entirely. This is safe as
enable_count is only ever written under rw_lock held for read (via
pwrseq_unit_enable()/disable(), reached only from pwrseq_power_on/off()),
so holding rw_lock for writing already excludes every other writer and
reader and the active-users WARN() stays race-free without state_lock.

Fixes: 249ebf3f65f8 ("power: sequencing: implement the pwrseq core")
Closes: https://sashiko.dev/#/patchset/20260616151049.1705503-1-vulab%40iscas.ac.cn
Link: https://patch.msgid.link/20260618-pwrseq-abba-deadlock-v1-1-943a3fd81c06@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
drivers/power/sequencing/core.c

index c1c2265160a33352ef86216347e1d175824d9d5f..02f42da915985339d3de507fc36dd158b0035a99 100644 (file)
@@ -543,15 +543,18 @@ void pwrseq_device_unregister(struct pwrseq_device *pwrseq)
        struct device *dev = &pwrseq->dev;
        struct pwrseq_target *target;
 
-       scoped_guard(mutex, &pwrseq->state_lock) {
+       scoped_guard(rwsem_write, &pwrseq_sem) {
                guard(rwsem_write)(&pwrseq->rw_lock);
 
+               /*
+                * Holding rw_lock for write excludes all power on/off callers
+                * (they hold it for read), so it's safe to read enable_count
+                * here without taking the state_lock.
+                */
                list_for_each_entry(target, &pwrseq->targets, list)
                        WARN(target->unit->enable_count,
                             "REMOVING POWER SEQUENCER WITH ACTIVE USERS\n");
 
-               guard(rwsem_write)(&pwrseq_sem);
-
                device_del(dev);
        }