Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1d915c56 authored by David Collins's avatar David Collins Committed by Greg Kroah-Hartman
Browse files

regulator: core: avoid regulator_resolve_supply() race condition 



[ Upstream commit eaa7995c529b54d68d97a30f6344cc6ca2f214a7 ]

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: default avatarDavid Collins <collinsd@codeaurora.org>
Link: https://lore.kernel.org/r/1610068562-4410-1-git-send-email-collinsd@codeaurora.org


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent fac13c1d

drivers/regulator/core.c

+28 −11
Original line number Diff line number Diff line
@@ -1567,23 +1567,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 resovle? */

	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;
@@ -1591,15 +1602,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);

	}
@@ -1613,7 +1627,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;

		}

	}
@@ -1621,13 +1636,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;

	}



	/* Cascade always-on state to supply */
@@ -1636,11 +1651,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 */