mirror of
https://github.com/linux-msm/laptops-kernel.git
synced 2026-08-13 14:19:53 -07:00
power: supply: charger-manager: register regulators before exposing sysfs
charger_manager_remove() and the err_reg_extcon probe error path free each
charger regulator with 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.
When charging is enabled, try_charger_enable(cm, false) in .remove() also
dereferences the freed handles directly. Both leave use-after-free windows.
Symmetrically, probe registers the sysfs entries (power_supply_register)
before acquiring the regulators (regulator_get, inside
charger_manager_register_extcon), so userspace can reach externally_control
before the regulators are available.
Split charger_manager_register_extcon() on the sync/async boundary:
charger_manager_get_regulators() (regulator_get only, no async producer)
now runs before power_supply_register() so sysfs is not live before
regulators are available, and charger_manager_register_extcon() keeps only
the extcon notifier/work setup, still after power_supply_register() so a
power_supply_register() failure cannot reach extcon setup. This keeps the
sysfs setup/teardown ordering symmetric without introducing an asynchronous
producer on the earlier probe-error path.
Move power_supply_unregister() and try_charger_enable(cm, false) ahead of
the regulator_put() loop on both teardown paths, and adjust err_reg_extcon
(power_supply_unregister() then fall through err_regulator for
regulator_put(); get_regulators self-rolls back on its own failure).
This does not address the separate extcon-notifier-driven deref of the same
handles, which needs its own synchronization design.
Found by an in-house static analysis tool.
Fixes: 3950c7865c ("charger-manager: Add support sysfs entry for charger")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
Link: https://patch.msgid.link/20260728030123.230202-1-fanwu01@zju.edu.cn
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
This commit is contained in:
committed by
Sebastian Reichel
parent
dfc859bb8d
commit
c57cb36f76
@@ -1014,6 +1014,29 @@ static int charger_extcon_init(struct charger_manager *cm,
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int charger_manager_get_regulators(struct charger_manager *cm)
|
||||
{
|
||||
struct charger_desc *desc = cm->desc;
|
||||
struct charger_regulator *charger;
|
||||
int i, ret;
|
||||
|
||||
for (i = 0; i < desc->num_charger_regulators; i++) {
|
||||
charger = &desc->charger_regulators[i];
|
||||
charger->consumer = regulator_get(cm->dev,
|
||||
charger->regulator_name);
|
||||
if (IS_ERR(charger->consumer)) {
|
||||
dev_err(cm->dev, "Cannot find charger(%s)\n",
|
||||
charger->regulator_name);
|
||||
ret = PTR_ERR(charger->consumer);
|
||||
while (i-- > 0)
|
||||
regulator_put(desc->charger_regulators[i].consumer);
|
||||
return ret;
|
||||
}
|
||||
charger->cm = cm;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* charger_manager_register_extcon - Register extcon device to receive state
|
||||
* of charger cable.
|
||||
@@ -1036,15 +1059,6 @@ static int charger_manager_register_extcon(struct charger_manager *cm)
|
||||
for (i = 0; i < desc->num_charger_regulators; i++) {
|
||||
charger = &desc->charger_regulators[i];
|
||||
|
||||
charger->consumer = regulator_get(cm->dev,
|
||||
charger->regulator_name);
|
||||
if (IS_ERR(charger->consumer)) {
|
||||
dev_err(cm->dev, "Cannot find charger(%s)\n",
|
||||
charger->regulator_name);
|
||||
return PTR_ERR(charger->consumer);
|
||||
}
|
||||
charger->cm = cm;
|
||||
|
||||
for (j = 0; j < charger->num_cables; j++) {
|
||||
struct charger_cable *cable = &charger->cables[j];
|
||||
|
||||
@@ -1580,13 +1594,23 @@ static int charger_manager_probe(struct platform_device *pdev)
|
||||
}
|
||||
psy_cfg.attr_grp = desc->sysfs_groups;
|
||||
|
||||
/*
|
||||
* Acquire charger regulators before exposing the sysfs entries, so
|
||||
* userspace cannot reach externally_control before the regulators
|
||||
* (and charger->cm) are available. Mirrors the order in remove().
|
||||
*/
|
||||
ret = charger_manager_get_regulators(cm);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
|
||||
cm->charger_psy = power_supply_register(&pdev->dev,
|
||||
&cm->charger_psy_desc,
|
||||
&psy_cfg);
|
||||
if (IS_ERR(cm->charger_psy)) {
|
||||
dev_err(&pdev->dev, "Cannot register charger-manager with name \"%s\"\n",
|
||||
cm->charger_psy_desc.name);
|
||||
return PTR_ERR(cm->charger_psy);
|
||||
ret = PTR_ERR(cm->charger_psy);
|
||||
goto err_regulator;
|
||||
}
|
||||
|
||||
/* Register extcon device for charger cable */
|
||||
@@ -1620,11 +1644,11 @@ static int charger_manager_probe(struct platform_device *pdev)
|
||||
return 0;
|
||||
|
||||
err_reg_extcon:
|
||||
power_supply_unregister(cm->charger_psy);
|
||||
err_regulator:
|
||||
for (i = 0; i < desc->num_charger_regulators; i++)
|
||||
regulator_put(desc->charger_regulators[i].consumer);
|
||||
|
||||
power_supply_unregister(cm->charger_psy);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
@@ -1642,12 +1666,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[] = {
|
||||
|
||||
Reference in New Issue
Block a user