mirror of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2025-09-04 20:19:47 +08:00
regulator: core: avoid regulator_resolve_supply() race condition
The final step in regulator_register() is to call
regulator_resolve_supply() for each registered regulator
(including the one in the process of being registered). The
regulator_resolve_supply() function first checks if rdev->supply
is NULL, then it performs various steps to try to find the supply.
If successful, rdev->supply is set inside of set_supply().
This procedure can encounter a race condition if two concurrent
tasks call regulator_register() near to each other on separate CPUs
and one of the regulators has rdev->supply_name specified. There
is currently nothing guaranteeing atomicity between the rdev->supply
check and set steps. Thus, both tasks can observe rdev->supply==NULL
in their regulator_resolve_supply() calls. This then results in
both creating a struct regulator for the supply. One ends up
actually stored in rdev->supply and the other is lost (though still
present in the supply's consumer_list).
Here is a kernel log snippet showing the issue:
[ 12.421768] gpu_cc_gx_gdsc: supplied by pm8350_s5_level
[ 12.425854] gpu_cc_gx_gdsc: supplied by pm8350_s5_level
[ 12.429064] debugfs: Directory 'regulator.4-SUPPLY' with parent
'17a00000.rsc:rpmh-regulator-gfxlvl-pm8350_s5_level'
already present!
Avoid this race condition by holding the rdev->mutex lock inside
of regulator_resolve_supply() while checking and setting
rdev->supply.
Signed-off-by: David Collins <collinsd@codeaurora.org>
Link: https://lore.kernel.org/r/1610068562-4410-1-git-send-email-collinsd@codeaurora.org
Signed-off-by: Mark Brown <broonie@kernel.org>
This commit is contained in:
parent
36836f5b37
commit
eaa7995c52
@ -1813,23 +1813,34 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||||||
{
|
{
|
||||||
struct regulator_dev *r;
|
struct regulator_dev *r;
|
||||||
struct device *dev = rdev->dev.parent;
|
struct device *dev = rdev->dev.parent;
|
||||||
int ret;
|
int ret = 0;
|
||||||
|
|
||||||
/* No supply to resolve? */
|
/* No supply to resolve? */
|
||||||
if (!rdev->supply_name)
|
if (!rdev->supply_name)
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
/* Supply already resolved? */
|
/* Supply already resolved? (fast-path without locking contention) */
|
||||||
if (rdev->supply)
|
if (rdev->supply)
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Recheck rdev->supply with rdev->mutex lock held to avoid a race
|
||||||
|
* between rdev->supply null check and setting rdev->supply in
|
||||||
|
* set_supply() from concurrent tasks.
|
||||||
|
*/
|
||||||
|
regulator_lock(rdev);
|
||||||
|
|
||||||
|
/* Supply just resolved by a concurrent task? */
|
||||||
|
if (rdev->supply)
|
||||||
|
goto out;
|
||||||
|
|
||||||
r = regulator_dev_lookup(dev, rdev->supply_name);
|
r = regulator_dev_lookup(dev, rdev->supply_name);
|
||||||
if (IS_ERR(r)) {
|
if (IS_ERR(r)) {
|
||||||
ret = PTR_ERR(r);
|
ret = PTR_ERR(r);
|
||||||
|
|
||||||
/* Did the lookup explicitly defer for us? */
|
/* Did the lookup explicitly defer for us? */
|
||||||
if (ret == -EPROBE_DEFER)
|
if (ret == -EPROBE_DEFER)
|
||||||
return ret;
|
goto out;
|
||||||
|
|
||||||
if (have_full_constraints()) {
|
if (have_full_constraints()) {
|
||||||
r = dummy_regulator_rdev;
|
r = dummy_regulator_rdev;
|
||||||
@ -1837,15 +1848,18 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||||||
} else {
|
} else {
|
||||||
dev_err(dev, "Failed to resolve %s-supply for %s\n",
|
dev_err(dev, "Failed to resolve %s-supply for %s\n",
|
||||||
rdev->supply_name, rdev->desc->name);
|
rdev->supply_name, rdev->desc->name);
|
||||||
return -EPROBE_DEFER;
|
ret = -EPROBE_DEFER;
|
||||||
|
goto out;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (r == rdev) {
|
if (r == rdev) {
|
||||||
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
|
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
|
||||||
rdev->desc->name, rdev->supply_name);
|
rdev->desc->name, rdev->supply_name);
|
||||||
if (!have_full_constraints())
|
if (!have_full_constraints()) {
|
||||||
return -EINVAL;
|
ret = -EINVAL;
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
r = dummy_regulator_rdev;
|
r = dummy_regulator_rdev;
|
||||||
get_device(&r->dev);
|
get_device(&r->dev);
|
||||||
}
|
}
|
||||||
@ -1859,7 +1873,8 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||||||
if (r->dev.parent && r->dev.parent != rdev->dev.parent) {
|
if (r->dev.parent && r->dev.parent != rdev->dev.parent) {
|
||||||
if (!device_is_bound(r->dev.parent)) {
|
if (!device_is_bound(r->dev.parent)) {
|
||||||
put_device(&r->dev);
|
put_device(&r->dev);
|
||||||
return -EPROBE_DEFER;
|
ret = -EPROBE_DEFER;
|
||||||
|
goto out;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1867,13 +1882,13 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||||||
ret = regulator_resolve_supply(r);
|
ret = regulator_resolve_supply(r);
|
||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
put_device(&r->dev);
|
put_device(&r->dev);
|
||||||
return ret;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
ret = set_supply(rdev, r);
|
ret = set_supply(rdev, r);
|
||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
put_device(&r->dev);
|
put_device(&r->dev);
|
||||||
return ret;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -1886,11 +1901,13 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
_regulator_put(rdev->supply);
|
_regulator_put(rdev->supply);
|
||||||
rdev->supply = NULL;
|
rdev->supply = NULL;
|
||||||
return ret;
|
goto out;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return 0;
|
out:
|
||||||
|
regulator_unlock(rdev);
|
||||||
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Internal regulator request function */
|
/* Internal regulator request function */
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user