Ping: [PATCH] EFI: adjust cfg file buffer freeing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.04.2026 13:51, Jan Beulich wrote:
> The boot services FreePages() needs passing the size. Since we allocated
> one more byte to put a trailing nul there, we also need to bump the size
> passed there. Make a small helper function to centralize this.
> 
> Note that there's no permanent memory leak because of the oversight: The
> allocation is done using EfiLoaderData, and all memory of that type is
> later reclaimed anyway.
> 
> Fixes: df75f77092c1 ("EFI: avoid OOB config file reads")
> Reported-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Marek, Daniel?

Thanks, Jan

> ---
> This is an alternative proposal to
> https://lists.xen.org/archives/html/xen-devel/2026-04/msg01044.html.
> 
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -778,6 +778,16 @@ static void __init efi_relocate_esrt(EFI
>   */
>  #include "efi-boot.h"
>  
> +static void __init free_cfg(void)
> +{
> +    if ( !cfg.need_to_free )
> +        return;
> +
> +    /* One extra byte was allocated to put a nul character there. */
> +    efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size + 1));
> +    cfg.need_to_free = false;
> +}
> +
>  void __init noreturn blexit(const CHAR16 *str)
>  {
>      if ( str )
> @@ -787,8 +797,7 @@ void __init noreturn blexit(const CHAR16
>      if ( !efi_bs )
>          efi_arch_halt();
>  
> -    if ( cfg.need_to_free )
> -        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
> +    free_cfg();
>      if ( kernel.need_to_free )
>          efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size));
>      if ( ramdisk.need_to_free )
> @@ -1557,11 +1566,7 @@ void EFIAPI __init noreturn efi_start(EF
>              name.s = get_value(&cfg, "global", "chain");
>              if ( !name.s )
>                  break;
> -            if ( cfg.need_to_free )
> -            {
> -                efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
> -                cfg.need_to_free = false;
> -            }
> +            free_cfg();
>              if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
>              {
>                  PrintStr(L"Chained configuration file '");
> @@ -1631,11 +1636,7 @@ void EFIAPI __init noreturn efi_start(EF
>  
>          efi_arch_cfg_file_late(loaded_image, dir_handle, section.s);
>  
> -        if ( cfg.need_to_free )
> -        {
> -            efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
> -            cfg.need_to_free = false;
> -        }
> +        free_cfg();
>  
>          if ( dir_handle )
>              dir_handle->Close(dir_handle);