[PATCH 1/1] boot: pxe_utils: avoid illegal memory access

[Heinrich Schuchardt]

We must not use an uninitialized pointer as target for snprintf() if
label->fdtdir == NULL.

Avoid a buffer overrun due to allocating len bytes but calling snprintf()
with a larger value dir_len.

Fixes: 935109cd9e97 ("boot: pxe_utils: Add extension board devicetree overlay support")
Addresses-Coverity-ID: 638558 Memory - illegal accesses (UNINIT)
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
---
Hello Kory,

Please, provide a test case for your patch
935109cd9e97 ("boot: pxe_utils: Add extension board devicetree overlay support")

It is unclear to me why you use

    overlay_dir = './' if label->fdtdir == "" and
    overlay_dir = "/" if label->fdtdir == NULL.

When opening a file in U-Boot './foo" and "/foo" anyway point to the same
file foo in the root directory.

Shouldn't we use the same value "/" in both cases?

Best regards

Heinrich
---
 boot/pxe_utils.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 038416203fc..e2bf44c776f 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -445,7 +445,7 @@ static void label_boot_extension(struct pxe_context *ctx,
 	struct fdt_header *working_fdt;
 	struct alist *extension_list;
 	int ret, dir_len, len;
-	char *overlay_dir;
+	char *overlay_dir, *fdtdir;
 	const char *slash;
 	ulong fdt_addr;
 
@@ -464,26 +464,25 @@ static void label_boot_extension(struct pxe_context *ctx,
 		return;
 
 	/* Use fdtdir for now as the overlay devicetree directory */
-	if (label->fdtdir) {
-		len = strlen(label->fdtdir);
-		if (!len)
-			slash = "./";
-		else if (label->fdtdir[len - 1] != '/')
-			slash = "/";
-		else
-			slash = "";
+	if (label->fdtdir)
+		fdtdir = label->fdtdir;
+	else
+		fdtdir = "/";
 
-		dir_len = strlen(label->fdtdir) + strlen(slash) + 1;
-		overlay_dir = calloc(1, len);
-		if (!overlay_dir)
-			return;
+	len = strlen(label->fdtdir);
+	if (!len)
+		slash = "./";
+	else if (label->fdtdir[len - 1] != '/')
+		slash = "/";
+	else
+		slash = "";
 
-		snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir,
-			 slash);
-	} else {
-		dir_len = 2;
-		snprintf(overlay_dir, dir_len, "/");
-	}
+	dir_len = len + strlen(slash) + 1;
+	overlay_dir = calloc(1, dir_len);
+	if (!overlay_dir)
+		return;
+
+	snprintf(overlay_dir, dir_len, "%s%s", fdtdir, slash);
 
 	alist_for_each(extension, extension_list) {
 		char *overlay_file;
-- 
2.51.0