All of lore.kernel.org
 help / color / mirror / Atom feed
From: cy_huang <u0084500@gmail.com>
To: broonie@kernel.org
Cc: lgirdwood@gmail.com, lee@kernel.org, matthias.bgg@gmail.com,
	yangyingliang@huawei.com, chiaen_wu@richtek.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	ChiYuan Huang <cy_huang@richtek.com>
Subject: [PATCH 1/2] regulator: mt6370: Fix potential UAF issue
Date: Wed, 30 Nov 2022 16:37:42 +0800	[thread overview]
Message-ID: <1669797463-24887-1-git-send-email-u0084500@gmail.com> (raw)

From: ChiYuan Huang <cy_huang@richtek.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@huawei.com>
Signed-off-by: ChiYuan Huang <cy_huang@richtek.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



WARNING: multiple messages have this Message-ID (diff)
From: cy_huang <u0084500@gmail.com>
To: broonie@kernel.org
Cc: lgirdwood@gmail.com, lee@kernel.org, matthias.bgg@gmail.com,
	yangyingliang@huawei.com, chiaen_wu@richtek.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	ChiYuan Huang <cy_huang@richtek.com>
Subject: [PATCH 1/2] regulator: mt6370: Fix potential UAF issue
Date: Wed, 30 Nov 2022 16:37:42 +0800	[thread overview]
Message-ID: <1669797463-24887-1-git-send-email-u0084500@gmail.com> (raw)

From: ChiYuan Huang <cy_huang@richtek.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@huawei.com>
Signed-off-by: ChiYuan Huang <cy_huang@richtek.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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2022-11-30  8:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-30  8:37 cy_huang [this message]
2022-11-30  8:37 ` [PATCH 1/2] regulator: mt6370: Fix potential UAF issue cy_huang
2022-11-30  8:37 ` [PATCH 2/2] regulator: mt6370: Switch to use dev_err_probe() helper cy_huang
2022-11-30  8:37   ` cy_huang
2022-12-01 11:43 ` [PATCH 1/2] regulator: mt6370: Fix potential UAF issue Mark Brown
2022-12-01 11:43   ` Mark Brown
2022-12-02  3:35   ` ChiYuan Huang
2022-12-02  3:35     ` ChiYuan Huang
2022-12-02 12:08     ` Mark Brown
2022-12-02 12:08       ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1669797463-24887-1-git-send-email-u0084500@gmail.com \
    --to=u0084500@gmail.com \
    --cc=broonie@kernel.org \
    --cc=chiaen_wu@richtek.com \
    --cc=cy_huang@richtek.com \
    --cc=lee@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=yangyingliang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.