Re: [PATCH] EFI: refine cfgfile buffer allocation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.09.2026 11:09, Jan Beulich wrote:
> Use of AllocateMaxAddress requires that the variable pointed to by the
> last argument of ->AllocatePages() is initialized. For cfgfile buffers we
> don't need AllocateMaxAddress though, at which point initialization of
> "addr" also isn't necessary anymore.
> 
> Mirror the lack of address constraint also to the main / central buffer
> allocation in read_file().
> 
> Fixes: df75f77092c1 ("EFI: avoid OOB config file reads")
> Assisted-by: Sashiko + Opus 5.0
> Reported-by: George Dunlap <dunlapg@xxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

May I please ask for maintainer feedback regarding this patch? If this
small variant is acceptable, I'd like to get it in. If the larger form
Ross suggested is going to be a requirement, I'd like to know so I can
plan the re-work accordingly.

Thanks, Jan

> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -878,8 +878,13 @@ static bool __init read_file(EFI_FILE_HA
>      what = L"Allocation";
>      file->addr = min(1UL << (32 + PAGE_SHIFT),
>                       HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> -    /* For config files allocate an extra byte to put a NUL there. */
> -    ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> +    /*
> +     * For config files allocate an extra byte to put a NUL there.  There's
> +     * also no constraint on addresses for them.
> +     */
> +    ret = efi_bs->AllocatePages(file != &cfg ? AllocateMaxAddress
> +                                             : AllocateAnyPages,
> +                                EfiLoaderData,
>                                  PFN_UP(size + (file == &cfg)), &file->addr);
>      if ( EFI_ERROR(ret) )
>          goto fail;
> @@ -931,7 +936,7 @@ static bool __init read_section(const EF
>      if ( file == &cfg && file->size && !iscntrl(file->str[file->size - 1]) )
>      {
>          EFI_PHYSICAL_ADDRESS addr;
> -        EFI_STATUS ret = efi_bs->AllocatePages(AllocateMaxAddress,
> +        EFI_STATUS ret = efi_bs->AllocatePages(AllocateAnyPages,
>                                                 EfiLoaderData,
>                                                 PFN_UP(file->size + 1), 
> &addr);
>