[PATCH v3 1/3] power: supply: core: prevent unregistering a power supply while a callback runs
From: Alexey Charkov
Date: Thu Sep 10 2026 - 12:17:10 EST
Once a power supply is registered, its callbacks can immediately start
firing from other contexts, such as external_power_changed() triggered by
the TCPM stack. If a power supply is unregistered while the callback is
still running, the driver data can already be freed when the callback
tries to access it, leading to a use-after-free.
This happens e.g. when the hardware bus carrying the power supply device
malfunctions (e.g. I2C is hogged down by another malfunctioning device)
immediately after the power supply is registered, and thus the core is
still processing the callbacks which were queued up when the driver
starts the removal, leading in some cases to a kernel crash, e.g.:
[ 11.645942] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005
[ 11.646751] Mem abort info:
[ 11.647006] ESR = 0x0000000096000004
[ 11.647338] EC = 0x25: DABT (current EL), IL = 32 bits
[ 11.647806] SET = 0, FnV = 0
[ 11.648077] EA = 0, S1PTW = 0
[ 11.648356] FSC = 0x04: level 0 translation fault
[ 11.648785] Data abort info:
[ 11.649041] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 11.649524] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 11.649981] GCS = 0, Overlay = 0, DirtyBit = 0
[ 11.650390] [0000000000000005] user address but active_mm is swapper
[ 11.650955] Internal error: Oops: 0000000096000004 [#1] SMP
[ 11.651460] Modules linked in:
[ 11.651742] CPU: 1 UID: 0 PID: 144 Comm: kworker/1:2 Not tainted 7.2.0-rc6-g62a9297af2cd #1 PREEMPT
[ 11.652553] Hardware name: Flipper One rev. F0B1C2 (DT)
[ 11.653024] Workqueue: events power_supply_changed_work
[ 11.653511] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 11.654135] pc : __power_supply_is_supplied_by+0x18/0x100
[ 11.654624] lr : __power_supply_am_i_supplied+0x40/0xb8
[ 11.655098] sp : ffff80008192bb30
[ 11.655399] x29: ffff80008192bb30 x28: 0000000000000000 x27: 0000000000000000
[ 11.656049] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
[ 11.656695] x23: ffff0000c19f4200 x22: ffffdb2232ba4ea8 x21: ffff80008192bc28
[ 11.657344] x20: ffff0000c1eef000 x19: ffff80008192bc18 x18: 00000000a0886e62
[ 11.657991] x17: 000000040044ffff x16: 04500072b5503510 x15: 0000000000000000
[ 11.658639] x14: 0000000000000000 x13: 0000000000000220 x12: 0000000000000000
[ 11.659286] x11: 0000000000000000 x10: ffff0000c1fdb2b0 x9 : ffffdb2232ba5590
[ 11.659934] x8 : 00000000e5b906e6 x7 : ffff0000c2502778 x6 : ffffdb22339793d0
[ 11.660581] x5 : ffff80008192bc18 x4 : ffff0000c19cbca0 x3 : 0000000000000000
[ 11.661228] x2 : ffff0000c1fdaf40 x1 : ffffffffffffffed x0 : ffff0000c1eef000
[ 11.661878] Call trace:
[ 11.662103] __power_supply_is_supplied_by+0x18/0x100 (P)
[ 11.662596] __power_supply_am_i_supplied+0x40/0xb8
[ 11.663040] psy_for_each_psy_cb+0x20/0x40
[ 11.663416] class_for_each_device+0x110/0x150
[ 11.663825] power_supply_am_i_supplied+0x68/0x100
[ 11.664262] bq257xx_external_power_changed+0x58/0x140
[ 11.664733] __power_supply_changed_work+0x60/0x80
[ 11.665170] psy_for_each_psy_cb+0x20/0x40
[ 11.665545] class_for_each_device+0x110/0x150
[ 11.665953] power_supply_changed_work+0x98/0x1b8
[ 11.666382] process_one_work+0x164/0x4c0
[ 11.666758] worker_thread+0x19c/0x320
[ 11.667104] kthread+0x138/0x150
[ 11.667408] ret_from_fork+0x10/0x20
[ 11.667744] Code: d503233f a9bd7bfd 910003fd a90153f3 (f9400c34)
[ 11.668294] ---[ end trace 0000000000000000 ]---
Add a read-write semaphore between external_power_changed() and
power_supply_unregister() to prevent the latter from returning (and thus
the driver from freeing its data) while the callback is still running.
Fixes: bc1540561c9e ("power_supply: Add API for safe access of power supply function attrs")
Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
---
drivers/power/supply/power_supply_core.c | 26 +++++++++++++++++++++++++-
include/linux/power_supply.h | 10 ++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 47e307709e5e..1279785645fb 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1530,6 +1530,19 @@ void power_supply_external_power_changed(struct power_supply *psy)
!psy->desc->external_power_changed)
return;
+ /*
+ * Keep power_supply_unregister() from returning, and thus from letting
+ * the driver's data be freed, while the callback is running. The
+ * ->removing check has to happen under the lock: on its own, just like
+ * the use_cnt check above, it only tells us the supply was still there
+ * when we looked, not that it still is by the time the callback
+ * dereferences its driver data.
+ */
+ guard(rwsem_read)(&psy->epc_sem);
+
+ if (psy->removing)
+ return;
+
psy->desc->external_power_changed(psy);
}
EXPORT_SYMBOL_GPL(power_supply_external_power_changed);
@@ -1774,6 +1787,7 @@ __power_supply_register(struct device *parent,
}
spin_lock_init(&psy->changed_lock);
+ init_rwsem(&psy->epc_sem);
init_rwsem(&psy->extensions_sem);
INIT_LIST_HEAD(&psy->extensions);
@@ -1914,7 +1928,17 @@ EXPORT_SYMBOL_GPL(devm_power_supply_register);
void power_supply_unregister(struct power_supply *psy)
{
WARN_ON(atomic_dec_return(&psy->use_cnt));
- psy->removing = true;
+
+ /*
+ * Publish ->removing so that no new ->external_power_changed() call can
+ * start, and wait via semaphore for one that is already running: it may
+ * be a supplier's changed_work, which cancel_work_sync() below does not
+ * cover, and it may still dereference driver data that the caller is
+ * about to free.
+ */
+ scoped_guard(rwsem_write, &psy->epc_sem)
+ psy->removing = true;
+
cancel_work_sync(&psy->changed_work);
cancel_delayed_work_sync(&psy->deferred_register_work);
sysfs_remove_link(&psy->dev.kobj, "powers");
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index e749d2189335..131cafded72f 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -351,6 +351,16 @@ struct power_supply {
bool removing;
atomic_t use_cnt;
struct power_supply_battery_info *battery_info;
+ /*
+ * Held for read while ->external_power_changed() runs, and for write by
+ * power_supply_unregister() when it publishes @removing, so that it
+ * waits for an in-flight callback to finish. Without this a driver's
+ * data, typically devm-allocated on its own device, can be freed while
+ * the callback is still using it.
+ * Must not be shared with extensions_sem: callbacks may read their own
+ * properties, which takes that one for read.
+ */
+ struct rw_semaphore epc_sem;
struct rw_semaphore extensions_sem; /* protects "extensions" */
struct list_head extensions;
#ifdef CONFIG_THERMAL
--
2.55.0