Re: [PATCH] boot: pxe_utils: Fix memory allocation issues in overlay_dir handling

[Quentin Schulz]

Hi Köry,

On 11/12/25 2:17 PM, Kory Maincent wrote:
From: "Kory Maincent (TI.com)" <kory.mainc...@bootlin.com>
Fix two memory allocation bugs in label_boot_extension():

1. When label->fdtdir is not set, overlay_dir was used without any
    memory allocation. Add the missing calloc() in the else branch.

2. When label->fdtdir is set, the allocation incorrectly used the
    'len' variable instead of 'dir_len'. The 'dir_len' variable is
    calculated to include the fdtdir length plus the trailing slash,
    while 'len' was only for the fdtdir length. This caused incorrect
    memory allocation size.

These issues could lead to memory corruption or undefined behavior when
processing device tree overlays via PXE boot.

Closes: https://lists.denx.de/pipermail/u-boot/2025-November/602892.html
Fixes: 935109cd9e97 ("boot: pxe_utils: Add extension board devicetree overlay 
support")
Signed-off-by: Kory Maincent (TI.com) <kory.mainc...@bootlin.com>
---
  boot/pxe_utils.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 038416203fc..7a64b6b97d4 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -474,7 +474,7 @@ static void label_boot_extension(struct pxe_context *ctx,
                        slash = "";
  
  		dir_len = strlen(label->fdtdir) + strlen(slash) + 1;
-               overlay_dir = calloc(1, len);
+               overlay_dir = calloc(1, dir_len);
                if (!overlay_dir)
                        return;
  
@@ -482,6 +482,10 @@ static void label_boot_extension(struct pxe_context *ctx,
                         slash);
        } else {
                dir_len = 2;
+               overlay_dir = calloc(1, dir_len);
+               if (!overlay_dir)
+                       return;
+
                snprintf(overlay_dir, dir_len, "/");
        }
  

I'm wondering if we couldn't make this easier to maintain by not having 
two calloc and snprintf calls?

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 038416203fc..6c1bf05cf66 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -474,17 +474,17 @@ static void label_boot_extension(struct 
pxe_context *ctx,
                        slash = "";

                dir_len = strlen(label->fdtdir) + strlen(slash) + 1;
-               overlay_dir = calloc(1, len);
-               if (!overlay_dir)
-                       return;
-
-               snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir,
-                        slash);
        } else {
                dir_len = 2;
-               snprintf(overlay_dir, dir_len, "/");
+               slash = "/";
        }

+       overlay_dir = calloc(1, dir_len);
+       if (!overlay_dir)
+               return;
+
+       snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir?: "", slash);
+
        alist_for_each(extension, extension_list) {
                char *overlay_file;
                ulong size;


Also, we probably want dir_len = len + strlen(slash) + 1 to avoid a 
second strlen on label->fdtdir (at the top of the git context here).

Finally, I'm wondering if the snprintf should not be dir_len - 1 
considering we calloc with enough room for the trailing NUL character? 
Or not have + 1 for dir_len and calloc with + 1.

Looks ok to me otherwise!

Cheers,
Quentin