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

[Kory Maincent]

On Wed, 12 Nov 2025 14:44:33 +0100
Quentin Schulz <quentin.schulz at cherry.de> wrote:

> 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 = "/";

In fact I think "./" should be the default one here.

>   	}
> 
> +	overlay_dir = calloc(1, dir_len);
> +	if (!overlay_dir)
> +		return;
> +
> +	snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir?: "", slash);

Yes, that's a good idea!

> 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).

Indeed!

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

No, snprintf second argument is the max size written including the null
character.

> Or not have + 1 for dir_len and calloc with + 1.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com