[PATCH master 1/3] environment: don't leak environment path buffer

[PATCH master 1/3] environment: don't leak environment path buffer

[Ahmad Fatoum]

LeakSanitizer on sandbox reports that we always leak the environment
path. Restructure the code, so this is avoided.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/barebox.c | 49 +++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/of/barebox.c b/drivers/of/barebox.c
index 560d9c0d15e0..ed5d171e43a8 100644
--- a/drivers/of/barebox.c
+++ b/drivers/of/barebox.c
@@ -15,60 +15,71 @@
 
 #define ENV_MNT_DIR "/boot"	/* If env on filesystem, where to mount */
 
-/* If dev describes a file on a fs, mount the fs and change devpath to
- * point to the file's path.  Otherwise leave devpath alone.  Does
- * nothing in env in a file support isn't enabled.  */
-static int environment_check_mount(struct device *dev, char **devpath)
+/* If dev describes a file on a fs, mount the fs and return a pointer
+ * to the file's path.  Otherwise return an error code or NULL if the
+ * device path should be used.
+ * Does nothing in env in a file support isn't enabled.
+ */
+static char *environment_check_mount(struct device *dev, const char *devpath)
 {
 	const char *filepath;
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_OF_BAREBOX_ENV_IN_FS))
-		return 0;
+		return NULL;
 
 	ret = of_property_read_string(dev->of_node, "file-path", &filepath);
 	if (ret == -EINVAL) {
 		/* No file-path so just use device-path */
-		return 0;
+		return NULL;
 	} else if (ret) {
 		/* file-path property exists, but has error */
 		dev_err(dev, "Problem with file-path property\n");
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	/* Get device env is on and mount it */
 	mkdir(ENV_MNT_DIR, 0777);
-	ret = mount(*devpath, "fat", ENV_MNT_DIR, NULL);
+	ret = mount(devpath, "fat", ENV_MNT_DIR, NULL);
 	if (ret) {
 		dev_err(dev, "Failed to load environment: mount %s failed (%d)\n",
-			*devpath, ret);
-		return ret;
+			devpath, ret);
+		return ERR_PTR(ret);
 	}
 
 	/* Set env to be in a file on the now mounted device */
 	dev_dbg(dev, "Loading default env from %s on device %s\n",
-		filepath, *devpath);
-	*devpath = basprintf("%s/%s", ENV_MNT_DIR, filepath);
-	return 0;
+		filepath, devpath);
+
+	return basprintf("%s/%s", ENV_MNT_DIR, filepath);
 }
 
 static int environment_probe(struct device *dev)
 {
-	char *path;
+	char *devpath, *filepath;
 	int ret;
 
-	ret = of_find_path(dev->of_node, "device-path", &path,
+	ret = of_find_path(dev->of_node, "device-path", &devpath,
 			   OF_FIND_PATH_FLAGS_BB);
 	if (ret)
 		return ret;
 
 	/* Do we need to mount a fs and find env there? */
-	ret = environment_check_mount(dev, &path);
-	if (ret)
+	filepath = environment_check_mount(dev, devpath);
+	if (IS_ERR(filepath)) {
+		free(devpath);
 		return ret;
+	}
 
-	dev_dbg(dev, "Setting default environment path to %s\n", path);
-	default_environment_path_set(path);
+	if (filepath)
+		free(devpath);
+	else
+		filepath = devpath;
+
+	dev_dbg(dev, "Setting default environment path to %s\n", filepath);
+	default_environment_path_set(filepath);
+
+	free(filepath);
 
 	return 0;
 }
-- 
2.39.2

[PATCH master 2/3] partition: fix use of uninitialized variable in error message

[Ahmad Fatoum]

The error message reports the partition number using the variable i, but
that's never initialized, as clang notices. Fix this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/partitions.c b/common/partitions.c
index 0b10377e7327..d46ed4080597 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -241,7 +241,7 @@ void partition_desc_init(struct partition_desc *pd, struct block_device *blk)
  */
 int parse_partition_table(struct block_device *blk)
 {
-	int i;
+	int i = 0;
 	int rc = 0;
 	struct partition *part;
 	struct partition_desc *pdesc;
@@ -259,6 +259,8 @@ int parse_partition_table(struct block_device *blk)
 				i, blk->cdev.name, rc);
 		if (rc != -ENODEV)
 			rc = 0;
+
+		i++;
 	}
 
 	partition_table_free(pdesc);
-- 
2.39.2

[PATCH master 3/3] of: fdt: harden against corrupted reserve map entries

[Ahmad Fatoum]

It's only safe to dereference r when dt_ptr_ok(fdt, r) determines that
the r object is within the bounds of fdt.

Commit 8a6b7db572c7 ("of: fdt: fix possibles overflows during
parsing of invalid DTs") had a first attempt at enforcing this, but
failed to do this for the very last element, so shift around the code,
so we only every dereference r when it's safe to do so.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/fdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 8dca41990c87..f56f5802bb73 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -69,14 +69,14 @@ static int of_reservemap_num_entries(const struct fdt_header *fdt)
 
 	r = (void *)fdt + be32_to_cpu(fdt->off_mem_rsvmap);
 
-	while (dt_ptr_ok(fdt, r) && r->size) {
+	while (dt_ptr_ok(fdt, r) && n < OF_MAX_RESERVE_MAP) {
+		if (!r->size)
+			return n;
 		n++;
 		r++;
-		if (n == OF_MAX_RESERVE_MAP)
-			return -EINVAL;
 	}
 
-	return r->size == 0 ? n : -ESPIPE;
+	return n == OF_MAX_RESERVE_MAP ? -EINVAL : -ESPIPE;
 }
 
 /**
-- 
2.39.2

Re: [PATCH master 1/3] environment: don't leak environment path buffer

[Sascha Hauer]

On Wed, 17 Jul 2024 09:29:11 +0200, Ahmad Fatoum wrote:
> LeakSanitizer on sandbox reports that we always leak the environment
> path. Restructure the code, so this is avoided.
> 
> 

Applied, thanks!

[1/3] environment: don't leak environment path buffer
      https://git.pengutronix.de/cgit/barebox/commit/?id=48cead76ce50 (link may not be stable)
[2/3] partition: fix use of uninitialized variable in error message
      https://git.pengutronix.de/cgit/barebox/commit/?id=8ff2477b5bb6 (link may not be stable)
[3/3] of: fdt: harden against corrupted reserve map entries
      https://git.pengutronix.de/cgit/barebox/commit/?id=002bb5e97fa9 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>