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 device *dev = rdev->dev.parent;
|
||||
int ret;
|
||||
int ret = 0;
|
||||
|
||||
/* No supply to resolve? */
|
||||
if (!rdev->supply_name)
|
||||
return 0;
|
||||
|
||||
/* Supply already resolved? */
|
||||
/* Supply already resolved? (fast-path without locking contention) */
|
||||
if (rdev->supply)
|
||||
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);
|
||||
if (IS_ERR(r)) {
|
||||
ret = PTR_ERR(r);
|
||||
|
||||
/* Did the lookup explicitly defer for us? */
|
||||
if (ret == -EPROBE_DEFER)
|
||||
return ret;
|
||||
goto out;
|
||||
|
||||
if (have_full_constraints()) {
|
||||
r = dummy_regulator_rdev;
|
||||
|
@ -1837,15 +1848,18 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||
} else {
|
||||
dev_err(dev, "Failed to resolve %s-supply for %s\n",
|
||||
rdev->supply_name, rdev->desc->name);
|
||||
return -EPROBE_DEFER;
|
||||
ret = -EPROBE_DEFER;
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
|
||||
if (r == rdev) {
|
||||
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
|
||||
rdev->desc->name, rdev->supply_name);
|
||||
if (!have_full_constraints())
|
||||
return -EINVAL;
|
||||
if (!have_full_constraints()) {
|
||||
ret = -EINVAL;
|
||||
goto out;
|
||||
}
|
||||
r = dummy_regulator_rdev;
|
||||
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 (!device_is_bound(r->dev.parent)) {
|
||||
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);
|
||||
if (ret < 0) {
|
||||
put_device(&r->dev);
|
||||
return ret;
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = set_supply(rdev, r);
|
||||
if (ret < 0) {
|
||||
put_device(&r->dev);
|
||||
return ret;
|
||||
goto out;
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -1886,11 +1901,13 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
|
|||
if (ret < 0) {
|
||||
_regulator_put(rdev->supply);
|
||||
rdev->supply = NULL;
|
||||
return ret;
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
out:
|
||||
regulator_unlock(rdev);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Internal regulator request function */
|
||||
|
|
Loading…
Reference in New Issue