Unverified Commit 8f3cbcd6 authored by ChiYuan Huang's avatar ChiYuan Huang Committed by Mark Brown
Browse files

regulator: core: Use different devices for resource allocation and DT lookup 

Following by the below discussion, there's the potential UAF issue
between regulator and mfd.
https://lore.kernel.org/all/20221128143601.1698148-1-yangyingliang@huawei.com/



From the analysis of Yingliang

CPU A				|CPU B
mt6370_probe()			|
  devm_mfd_add_devices()	|
				|mt6370_regulator_probe()
				|  regulator_register()
				|    //allocate init_data and add it to devres
				|    regulator_of_get_init_data()
i2c_unregister_device()		|
  device_del()			|
    devres_release_all()	|
      // init_data is freed	|
      release_nodes()		|
				|  // using init_data causes UAF
				|  regulator_register()

It's common to use mfd core to create child device for the regulator.
In order to do the DT lookup for init data, the child that registered
the regulator would pass its parent as the parameter. And this causes
init data resource allocated to its parent, not itself. The issue happen
when parent device is going to release and regulator core is still doing
some operation of init data constraint for the regulator of child device.

To fix it, this patch expand 'regulator_register' API to use the
different devices for init data allocation and DT lookup.

Reported-by: default avatarYang Yingliang <yangyingliang@huawei.com>
Signed-off-by: default avatarChiYuan Huang <cy_huang@richtek.com>
Link: https://lore.kernel.org/r/1670311341-32664-1-git-send-email-u0084500@gmail.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 2a17ddfd

drivers/platform/x86/intel/int3472/clk_and_regulator.c

+2 −1
Original line number Original line Diff line number Diff line
@@ -185,7 +185,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472, 
	cfg.init_data = &init_data;

	cfg.init_data = &init_data;

	cfg.ena_gpiod = int3472->regulator.gpio;

	cfg.ena_gpiod = int3472->regulator.gpio;





	int3472->regulator.rdev = regulator_register(&int3472->regulator.rdesc,

	int3472->regulator.rdev = regulator_register(int3472->dev,

						     &int3472->regulator.rdesc,

						     &cfg);

						     &cfg);

	if (IS_ERR(int3472->regulator.rdev)) {

	if (IS_ERR(int3472->regulator.rdev)) {

		ret = PTR_ERR(int3472->regulator.rdev);

		ret = PTR_ERR(int3472->regulator.rdev);

drivers/regulator/core.c

+4 −4
Original line number Original line Diff line number Diff line
@@ -5409,6 +5409,7 @@ static struct regulator_coupler generic_regulator_coupler = { 




/**

/**

 * regulator_register - register regulator

 * regulator_register - register regulator

 * @dev: the device that drive the regulator

 * @regulator_desc: regulator to register

 * @regulator_desc: regulator to register

 * @cfg: runtime configuration for regulator

 * @cfg: runtime configuration for regulator

 *

 *
@@ -5417,7 +5418,8 @@ static struct regulator_coupler generic_regulator_coupler = { 
 * or an ERR_PTR() on error.

 * or an ERR_PTR() on error.

 */

 */

struct regulator_dev *

struct regulator_dev *

regulator_register(const struct regulator_desc *regulator_desc,

regulator_register(struct device *dev,

		   const struct regulator_desc *regulator_desc,

		   const struct regulator_config *cfg)

		   const struct regulator_config *cfg)

{

{

	const struct regulator_init_data *init_data;

	const struct regulator_init_data *init_data;
@@ -5426,7 +5428,6 @@ regulator_register(const struct regulator_desc *regulator_desc, 
	struct regulator_dev *rdev;

	struct regulator_dev *rdev;

	bool dangling_cfg_gpiod = false;

	bool dangling_cfg_gpiod = false;

	bool dangling_of_gpiod = false;

	bool dangling_of_gpiod = false;

	struct device *dev;

	int ret, i;

	int ret, i;

	bool resolved_early = false;

	bool resolved_early = false;
@@ -5439,8 +5440,7 @@ regulator_register(const struct regulator_desc *regulator_desc, 
		goto rinse;

		goto rinse;

	}

	}





	dev = cfg->dev;

	WARN_ON(!dev || !cfg->dev);

	WARN_ON(!dev);





	if (regulator_desc->name == NULL || regulator_desc->ops == NULL) {

	if (regulator_desc->name == NULL || regulator_desc->ops == NULL) {

		ret = -EINVAL;

		ret = -EINVAL;

drivers/regulator/devres.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -415,7 +415,7 @@ struct regulator_dev *devm_regulator_register(struct device *dev, 
	if (!ptr)

	if (!ptr)

		return ERR_PTR(-ENOMEM);

		return ERR_PTR(-ENOMEM);





	rdev = regulator_register(regulator_desc, config);

	rdev = regulator_register(dev, regulator_desc, config);

	if (!IS_ERR(rdev)) {

	if (!IS_ERR(rdev)) {

		*ptr = rdev;

		*ptr = rdev;

		devres_add(dev, ptr);

		devres_add(dev, ptr);

drivers/regulator/of_regulator.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -505,7 +505,7 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev, 
	struct device_node *child;

	struct device_node *child;

	struct regulator_init_data *init_data = NULL;

	struct regulator_init_data *init_data = NULL;





	child = regulator_of_get_init_node(dev, desc);

	child = regulator_of_get_init_node(config->dev, desc);

	if (!child)

	if (!child)

		return NULL;

		return NULL;

drivers/regulator/stm32-vrefbuf.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -210,7 +210,7 @@ static int stm32_vrefbuf_probe(struct platform_device *pdev) 
						      pdev->dev.of_node,

						      pdev->dev.of_node,

						      &stm32_vrefbuf_regu);

						      &stm32_vrefbuf_regu);





	rdev = regulator_register(&stm32_vrefbuf_regu, &config);

	rdev = regulator_register(&pdev->dev, &stm32_vrefbuf_regu, &config);

	if (IS_ERR(rdev)) {

	if (IS_ERR(rdev)) {

		ret = PTR_ERR(rdev);

		ret = PTR_ERR(rdev);

		dev_err(&pdev->dev, "register failed with error %d\n", ret);

		dev_err(&pdev->dev, "register failed with error %d\n", ret);