[PATCH] power: supply: charger-manager: tear down sysfs and disable charging before freeing regulators

Fan Wu <[email protected]>
Newsgroups org.kernel.vger.linux-pm,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
charger_manager_remove() and the err_reg_extcon probe error path call
regulator_put() before tearing down the power_supply sysfs entries
(power_supply_unregister()). charger_manager_remove() also calls
try_charger_enable(cm, false) after the regulator_put() loop. A concurrent
write to a charger's externally_control sysfs attribute that lands between
regulator_put() and power_supply_unregister() can run
charger_externally_control_store() and call try_charger_enable(), which,
when charging is enabled, dereferences the already-freed consumer handle
via regulator_enable(), regulator_disable(), regulator_is_enabled() and
regulator_force_disable(). When charging is enabled, try_charger_enable(cm,
false) in .remove() also dereferences the freed handles directly. Both
leave use-after-free windows.

Move power_supply_unregister() ahead of the regulator_put() loop on both
paths. It ends in device_unregister(), which removes the sysfs attribute
groups and drains active kernfs operations, so on return no
externally_control store can be running or start again; it does not touch
the regulator consumers, and psy->desc references the devm-managed
cm->charger_psy_desc, which outlives the call. Move try_charger_enable(cm,
false) before the regulator_put() loop in .remove() as well, so the
synchronous disable also runs on valid handles.

This does not address the separate extcon-notifier path, which needs its
own synchronization design.

Found by an in-house static analysis tool.

Fixes: 3950c7865cd7 ("charger-manager: Add support sysfs entry for charger")
Cc: [email protected]
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <[email protected]>
---
 drivers/power/supply/charger-manager.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index c49e0e4d02f7..c1df512f32a8 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1622,11 +1622,11 @@ static int charger_manager_probe(struct platform_device *pdev)
 	return 0;
 
 err_reg_extcon:
+	power_supply_unregister(cm->charger_psy);
+
 	for (i = 0; i < desc->num_charger_regulators; i++)
 		regulator_put(desc->charger_regulators[i].consumer);
 
-	power_supply_unregister(cm->charger_psy);
-
 	return ret;
 }
 
@@ -1644,12 +1644,12 @@ static void charger_manager_remove(struct platform_device *pdev)
 	cancel_work_sync(&setup_polling);
 	cancel_delayed_work_sync(&cm_monitor_work);
 
-	for (i = 0 ; i < desc->num_charger_regulators ; i++)
-		regulator_put(desc->charger_regulators[i].consumer);
+	try_charger_enable(cm, false);
 
 	power_supply_unregister(cm->charger_psy);
 
-	try_charger_enable(cm, false);
+	for (i = 0 ; i < desc->num_charger_regulators ; i++)
+		regulator_put(desc->charger_regulators[i].consumer);
 }
 
 static const struct platform_device_id charger_manager_id[] = {
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.