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

[ Upstream commit 8f3cbcd6b440032ebc7f7d48a1689dcc70a4eb98 ]

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: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Link: https://lore.kernel.org/r/1670311341-32664-1-git-send-email-u0084500@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
index 1cf958983e8680565d407c4b094b9c1d2505a833..b2342b3d78c7234303bb6155005f00b82c797cea 100644 (file)
--- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
+++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
@@ -185,7 +185,8 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
        cfg.init_data = &init_data;
        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);
        if (IS_ERR(int3472->regulator.rdev)) {
                ret = PTR_ERR(int3472->regulator.rdev);

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 15677f5dcd994e69bc40904f8a0fd66ffa4d7703..8b567664f8120fa83de9459e009bcc479cbc8f22 100644 (file)
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5404,6 +5404,7 @@ static struct regulator_coupler generic_regulator_coupler = {
 
 /**
  * regulator_register - register regulator
+ * @dev: the device that drive the regulator
  * @regulator_desc: regulator to register
  * @cfg: runtime configuration for regulator
  *
@@ -5412,7 +5413,8 @@ static struct regulator_coupler generic_regulator_coupler = {
  * or an ERR_PTR() on error.
  */
 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_init_data *init_data;
@@ -5421,7 +5423,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
        struct regulator_dev *rdev;
        bool dangling_cfg_gpiod = false;
        bool dangling_of_gpiod = false;
-       struct device *dev;
        int ret, i;
        bool resolved_early = false;
 
@@ -5434,8 +5435,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
                goto rinse;
        }
 
-       dev = cfg->dev;
-       WARN_ON(!dev);
+       WARN_ON(!dev || !cfg->dev);
 
        if (regulator_desc->name == NULL || regulator_desc->ops == NULL) {
                ret = -EINVAL;

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 3265e75e97ab4cab1dfd4eb16b94c40456c2ee35..5c7ff9b3e8a79b694f800e6a07679bd0c7106d9e 100644 (file)
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -385,7 +385,7 @@ struct regulator_dev *devm_regulator_register(struct device *dev,
        if (!ptr)
                return ERR_PTR(-ENOMEM);
 
-       rdev = regulator_register(regulator_desc, config);
+       rdev = regulator_register(dev, regulator_desc, config);
        if (!IS_ERR(rdev)) {
                *ptr = rdev;
                devres_add(dev, ptr);

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 0aff1c2886b5dd425e4a20299a47dc7833fb64ea..cd726d4e8fbfb7da646f2cd3915ad74a19f66c3b 100644 (file)
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -505,7 +505,7 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
        struct device_node *child;
        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)
                return NULL;
 

diff --git a/drivers/regulator/stm32-vrefbuf.c b/drivers/regulator/stm32-vrefbuf.c
index 30ea3bc8ca1921ced12c21c8c418475e6864db1b..7a454b7b6eab9958ad28ef2ba09739f3b893ce2c 100644 (file)
--- a/drivers/regulator/stm32-vrefbuf.c
+++ b/drivers/regulator/stm32-vrefbuf.c
@@ -210,7 +210,7 @@ static int stm32_vrefbuf_probe(struct platform_device *pdev)
                                                      pdev->dev.of_node,
                                                      &stm32_vrefbuf_regu);
 
-       rdev = regulator_register(&stm32_vrefbuf_regu, &config);
+       rdev = regulator_register(&pdev->dev, &stm32_vrefbuf_regu, &config);
        if (IS_ERR(rdev)) {
                ret = PTR_ERR(rdev);
                dev_err(&pdev->dev, "register failed with error %d\n", ret);

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index f9a7461e72b80ef345ef3165262ac82bce201ed7..d3b4a3d4514aba340650c673b8a34c05132cb348 100644 (file)
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -687,7 +687,8 @@ static inline int regulator_err2notif(int err)
 
 
 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 *config);
 struct regulator_dev *
 devm_regulator_register(struct device *dev,