[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1669797463-24887-1-git-send-email-u0084500@gmail.com>
Date: Wed, 30 Nov 2022 16:37:42 +0800
From: cy_huang <u0084500@...il.com>
To: broonie@...nel.org
Cc: lgirdwood@...il.com, lee@...nel.org, matthias.bgg@...il.com,
yangyingliang@...wei.com, chiaen_wu@...htek.com,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org,
ChiYuan Huang <cy_huang@...htek.com>
Subject: [PATCH 1/2] regulator: mt6370: Fix potential UAF issue
From: ChiYuan Huang <cy_huang@...htek.com>
Following by the below patch, there's potential UAF issue.
https://lore.kernel.org/all/20221128143601.1698148-1-yangyingliang@huawei.com/
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()
The original code uses i2c dev as the parent in order to reuse
the 'regulator_of_get_init_data'. But this will cause regulation
constraint devres attached to i2c dev, not the mfd cell platform
device.
Use 'of_regulator_match' to directly parse regulation constraint from
parent dev node. Correct all regulator devs parent back to the platform
device itself.
Fixes: 8171c93bac1b ("regulator: mt6370: Add mt6370 DisplayBias and VibLDO support")
Reported-by: Yang Yingliang <yangyingliang@...wei.com>
Signed-off-by: ChiYuan Huang <cy_huang@...htek.com>
---
drivers/regulator/mt6370-regulator.c | 61 ++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/drivers/regulator/mt6370-regulator.c b/drivers/regulator/mt6370-regulator.c
index e73f5a4..c2b589a 100644
--- a/drivers/regulator/mt6370-regulator.c
+++ b/drivers/regulator/mt6370-regulator.c
@@ -11,6 +11,7 @@
#include <linux/regmap.h>
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
enum {
MT6370_IDX_DSVBOOST = 0,
@@ -183,8 +184,6 @@ static int mt6370_of_parse_cb(struct device_node *np,
static const struct regulator_desc mt6370_regulator_descs[] = {
{
.name = "mt6370-dsv-vbst",
- .of_match = of_match_ptr("dsvbst"),
- .regulators_node = of_match_ptr("regulators"),
.id = MT6370_IDX_DSVBOOST,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
@@ -200,8 +199,6 @@ static const struct regulator_desc mt6370_regulator_descs[] = {
},
{
.name = "mt6370-dsv-vpos",
- .of_match = of_match_ptr("dsvpos"),
- .regulators_node = of_match_ptr("regulators"),
.id = MT6370_IDX_DSVPOS,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
@@ -224,8 +221,6 @@ static const struct regulator_desc mt6370_regulator_descs[] = {
},
{
.name = "mt6370-dsv-vneg",
- .of_match = of_match_ptr("dsvneg"),
- .regulators_node = of_match_ptr("regulators"),
.id = MT6370_IDX_DSVNEG,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
@@ -248,8 +243,6 @@ static const struct regulator_desc mt6370_regulator_descs[] = {
},
{
.name = "mt6370-vib-ldo",
- .of_match = of_match_ptr("vibldo"),
- .regulators_node = of_match_ptr("regulators"),
.id = MT6370_IDX_VIBLDO,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
@@ -320,23 +313,57 @@ static int mt6370_regulator_irq_register(struct mt6370_priv *priv)
return 0;
}
+static struct of_regulator_match mt6370_regulator_match[MT6370_MAX_IDX] = {
+ [MT6370_IDX_DSVBOOST] = { .name = "dsvbst" },
+ [MT6370_IDX_DSVPOS] = { .name = "dsvpos" },
+ [MT6370_IDX_DSVNEG] = { .name = "dsvneg" },
+ [MT6370_IDX_VIBLDO] = { .name = "vibldo" },
+};
+
static int mt6370_regualtor_register(struct mt6370_priv *priv)
{
struct regulator_dev *rdev;
- struct regulator_config cfg = {};
struct device *parent = priv->dev->parent;
- int i;
+ struct device *dev = priv->dev;
+ struct device_node *regulator_np;
+ int i, ret;
+
+ regulator_np = of_get_child_by_name(parent->of_node, "regulators");
+ if (!regulator_np) {
+ dev_err(dev, "Could not find parent 'regulators' node\n");
+ return -ENODEV;
+ }
+
+ ret = of_regulator_match(dev, regulator_np, mt6370_regulator_match,
+ ARRAY_SIZE(mt6370_regulator_match));
- cfg.dev = parent;
- cfg.driver_data = priv;
+ of_node_put(regulator_np);
+
+ if (ret < 0) {
+ dev_err(dev, "Error parsing regulator init data: %d\n", ret);
+ return ret;
+ }
for (i = 0; i < MT6370_MAX_IDX; i++) {
- rdev = devm_regulator_register(priv->dev,
- mt6370_regulator_descs + i,
- &cfg);
+ const struct regulator_desc *desc = mt6370_regulator_descs + i;
+ struct regulator_config cfg = {};
+
+ cfg.dev = dev;
+ cfg.driver_data = priv;
+ cfg.init_data = mt6370_regulator_match[i].init_data;
+ cfg.of_node = mt6370_regulator_match[i].of_node;
+
+ if (cfg.of_node && desc->of_parse_cb) {
+ ret = desc->of_parse_cb(cfg.of_node, desc, &cfg);
+ if (ret) {
+ dev_err(dev, "Failed in of_parse_cb\n");
+ return ret;
+ }
+ }
+
+ rdev = devm_regulator_register(dev, desc, &cfg);
if (IS_ERR(rdev)) {
- dev_err(priv->dev,
- "Failed to register (%d) regulator\n", i);
+ dev_err(dev, "Failed to register (%d) regulator\n", i);
return PTR_ERR(rdev);
}
--
2.7.4
Powered by blists - more mailing lists