lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240822072047.3097740-4-wenst@chromium.org>
Date: Thu, 22 Aug 2024 15:20:46 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>
Cc: Chen-Yu Tsai <wenst@...omium.org>,
	linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: [PATCH 3/3] regulator: Fully clean up on error in of_regulator_bulk_get_all()

Currently in of_regulator_bulk_get_all(), if any regulator request
fails, the error path releases all regulators already requested,
but leaves the |struct regulator_bulk_data| memory to the caller
to free, and also leaves the regulator consumer pointers dangling.
The latter behavior is not documented, and may not be what the
caller is expecting.

Instead, explicitly clean up everything on error, and make it clear
that the result pointer is only update if the whole request succeeds.

Signed-off-by: Chen-Yu Tsai <wenst@...omium.org>
---
 drivers/regulator/of_regulator.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 86b680adbf01..d557f7b1ec7c 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -747,19 +747,19 @@ static int is_supply_name(const char *name)
  * This helper function allows drivers to get several regulator
  * consumers in one operation.  If any of the regulators cannot be
  * acquired then any regulators that were allocated will be freed
- * before returning to the caller.
+ * before returning to the caller, and @consumers will not be
+ * changed.
  */
 int of_regulator_bulk_get_all(struct device *dev, struct device_node *np,
 			      struct regulator_bulk_data **consumers)
 {
 	int num_consumers = 0;
 	struct regulator *tmp;
+	struct regulator_bulk_data *_consumers = NULL;
 	struct property *prop;
 	int i, n = 0, ret;
 	char name[64];
 
-	*consumers = NULL;
-
 	/*
 	 * first pass: get numbers of xxx-supply
 	 * second pass: fill consumers
@@ -769,7 +769,7 @@ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np,
 		i = is_supply_name(prop->name);
 		if (i == 0)
 			continue;
-		if (!*consumers) {
+		if (!_consumers) {
 			num_consumers++;
 			continue;
 		} else {
@@ -780,25 +780,28 @@ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np,
 				ret = PTR_ERR(tmp);
 				goto error;
 			}
-			(*consumers)[n].consumer = tmp;
+			_consumers[n].consumer = tmp;
 			n++;
 			continue;
 		}
 	}
-	if (*consumers)
+	if (_consumers) {
+		*consumers = _consumers;
 		return num_consumers;
+	}
 	if (num_consumers == 0)
 		return 0;
-	*consumers = kmalloc_array(num_consumers,
+	_consumers = kmalloc_array(num_consumers,
 				   sizeof(struct regulator_bulk_data),
 				   GFP_KERNEL);
-	if (!*consumers)
+	if (!_consumers)
 		return -ENOMEM;
 	goto restart;
 
 error:
 	while (--n >= 0)
-		regulator_put(consumers[n]->consumer);
+		regulator_put(_consumers[n].consumer);
+	kfree(_consumers);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_regulator_bulk_get_all);
-- 
2.46.0.184.g6999bdac58-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ