| Message ID | 20200929181717.1596965-2-hudson@trmm.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi: Unified Xen hypervisor/kernel/initrd images | expand |
On 29.09.2020 20:17, Trammell Hudson wrote: > The config file, kernel, initrd, etc should only be freed if they > are allocated with the UEFI allocator. On x86 the ucode, and on > ARM the dtb, are also marked as need_to_free. > > Signed-off-by: Trammell Hudson <hudson@trmm.net> non-Arm parts: Reviewed-by: Jan Beulich <jbeulich@suse.com> > --- a/xen/arch/arm/efi/efi-boot.h > +++ b/xen/arch/arm/efi/efi-boot.h > @@ -546,7 +546,7 @@ static void __init efi_arch_cpu(void) > > static void __init efi_arch_blexit(void) > { > - if ( dtbfile.addr && dtbfile.size ) > + if ( dtbfile.need_to_free ) > efi_bs->FreePages(dtbfile.addr, PFN_UP(dtbfile.size)); > if ( memmap ) > efi_bs->FreePool(memmap); I'm afraid this isn't enough of a change for Arm, due to what fdt_increase_size() may do. Jan
On Wednesday, September 30, 2020 2:49 AM, Jan Beulich <jbeulich@suse.com> wrote: > On 29.09.2020 20:17, Trammell Hudson wrote: > > - if ( dtbfile.addr && dtbfile.size ) > > - if ( dtbfile.need_to_free ) > > efi_bs->FreePages(dtbfile.addr, PFN_UP(dtbfile.size)); > > if ( memmap ) > > efi_bs->FreePool(memmap); > > > > I'm afraid this isn't enough of a change for Arm, due to what > fdt_increase_size() may do. You're right. It looks like there are also potential memory leaks in the fdt_increase_size() error paths as well. I've added free calls in v8. -- Trammell
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index 27dd0b1a94..82f2fc7685 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -546,7 +546,7 @@ static void __init efi_arch_cpu(void) static void __init efi_arch_blexit(void) { - if ( dtbfile.addr && dtbfile.size ) + if ( dtbfile.need_to_free ) efi_bs->FreePages(dtbfile.addr, PFN_UP(dtbfile.size)); if ( memmap ) efi_bs->FreePool(memmap); diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index eef3f52789..1025000afd 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -689,7 +689,7 @@ static void __init efi_arch_cpu(void) static void __init efi_arch_blexit(void) { - if ( ucode.addr ) + if ( ucode.need_to_free ) efi_bs->FreePages(ucode.addr, PFN_UP(ucode.size)); } diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 157fe0e8c5..c2ce0c7294 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -102,6 +102,7 @@ union string { struct file { UINTN size; + bool need_to_free; union { EFI_PHYSICAL_ADDRESS addr; char *str; @@ -280,13 +281,13 @@ void __init noreturn blexit(const CHAR16 *str) if ( !efi_bs ) efi_arch_halt(); - if ( cfg.addr ) + if ( cfg.need_to_free ) efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size)); - if ( kernel.addr ) + if ( kernel.need_to_free ) efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size)); - if ( ramdisk.addr ) + if ( ramdisk.need_to_free ) efi_bs->FreePages(ramdisk.addr, PFN_UP(ramdisk.size)); - if ( xsm.addr ) + if ( xsm.need_to_free ) efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size)); efi_arch_blexit(); @@ -581,6 +582,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name, } else { + file->need_to_free = true; file->size = size; if ( file != &cfg ) {
The config file, kernel, initrd, etc should only be freed if they are allocated with the UEFI allocator. On x86 the ucode, and on ARM the dtb, are also marked as need_to_free. Signed-off-by: Trammell Hudson <hudson@trmm.net> --- xen/arch/arm/efi/efi-boot.h | 2 +- xen/arch/x86/efi/efi-boot.h | 2 +- xen/common/efi/boot.c | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-)