[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.maincent at 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.maincent at 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