[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.schucha...@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