[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <552cf43e8ce7c7e430cd18feb20efb1dee2350e0.camel@gentoo.org>
Date: Thu, 04 Dec 2025 21:40:09 +0000
From: James Le Cuirot <chewi@...too.org>
To: Ard Biesheuvel <ardb@...nel.org>, Ingo Molnar <mingo@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, Thomas Gleixner
<tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, Dave Hansen
<dave.hansen@...ux.intel.com>, "H . Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86: fix oops caused by old EFI info on kexec boot
On Thu, 2025-12-04 at 00:01 +0100, Ard Biesheuvel wrote:
> On Wed, 3 Dec 2025 at 23:57, Ard Biesheuvel <ardb@...nel.org> wrote:
> >
> > On Wed, 3 Dec 2025 at 19:13, Ingo Molnar <mingo@...nel.org> wrote:
> > >
> > > * James Le Cuirot <chewi@...too.org> wrote:
> > >
> > > > kexec on x86 passes initrd details via the boot_params. If no initrd is
> > > > supplied, then ramdisk_size is 0. When determining whether to reserve
> > > > memory for the initrd on the subsequent boot, ramdisk_size being 0
> > > > causes the logic to fall back to phys_initrd_start and phys_initrd_size
> > > > set from the EFI tables in efi.c. This is stale information from the
> > > > initial boot. The system continues to boot and has even been seen to
> > > > function under heavy load for days, but allocating very large amounts of
> > > > memory reliably triggers an oops rather than the OOM killer.
> > > >
> > > > BUG: kernel NULL pointer dereference, address: 0000000000000008
> > > > #PF: supervisor write access in kernel mode
> > > > #PF: error_code(0x0002) - not-present page
> > > > PGD 0 P4D 0
> > > > Oops: Oops: 0002 [#1] SMP NOPTI
> > > >
> > > > This issue was introduced in f4dc7fffa9873db50ec25624572f8217a6225de8
> > > > when the EFI stub initrd loading was unified between architectures.
> > > >
> > > > Avoid the issue by checking whether the bootloader is not kexec before
> > > > falling back to the EFI table values.
> > > >
> > > > I strongly suspect this also affects other architectures. A different
> > > > fix would be required there, and I do have a fix in mind, but I was
> > > > unable to reproduce the issue under QEMU's aarch64 virt machine. I think
> > > > this is at least partly because it relies on ACPI while kexec passes the
> > > > initd details via the device tree.
> > > >
> > > > Signed-off-by: James Le Cuirot <chewi@...too.org>
> > > > ---
> > > > arch/x86/kernel/setup.c | 6 ++++--
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > > > index 1b2edd07a3e1..8aa65daf121f 100644
> > > > --- a/arch/x86/kernel/setup.c
> > > > +++ b/arch/x86/kernel/setup.c
> > > > @@ -300,7 +300,8 @@ static u64 __init get_ramdisk_image(void)
> > > >
> > > > ramdisk_image |= (u64)boot_params.ext_ramdisk_image << 32;
> > > >
> > > > - if (ramdisk_image == 0)
> > > > + /* Don't fall back for kexec as phys_initrd_start will be stale */
> > > > + if (ramdisk_image == 0 && (boot_params.hdr.type_of_loader >> 4) != 0xD)
> > > > ramdisk_image = phys_initrd_start;
> > > >
> > > > return ramdisk_image;
> > > > @@ -311,7 +312,8 @@ static u64 __init get_ramdisk_size(void)
> > > >
> > > > ramdisk_size |= (u64)boot_params.ext_ramdisk_size << 32;
> > > >
> > > > - if (ramdisk_size == 0)
> > > > + /* Don't fall back for kexec as phys_initrd_start will be stale */
> > > > + if (ramdisk_size == 0 && (boot_params.hdr.type_of_loader >> 4) != 0xD)
> > > > ramdisk_size = phys_initrd_size;
> > >
> > > Yeah, so this looks like a good fix - but please let's
> > > introduce some sort of enum for the bootloader IDs
> > > in arch/x86/include/uapi/asm/bootparam.h, I had to search
> > > way too long to figure out what 0xD is and where it
> > > was defined :-)
> > >
> > > Also, please introduce a "x86_bootloader_is_kexec()" kind
> > > of helper inline function as well.
> > >
> >
> > It might be better to fix this in the generic EFI code, and simply
> > wipe the EFI config table that the EFI stub created to pass the initrd
> > info. That way, it works for all architectures, and there is no need
> > for special x86 hacks.
>
> I.e.,
>
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -818,6 +818,7 @@
> if (tbl) {
> phys_initrd_start = tbl->base;
> phys_initrd_size = tbl->size;
> + tbl->base = tbl->size = 0;
> early_memunmap(tbl, sizeof(*tbl));
> }
> }
I can confirm that this fixes the problem. I had considered wiping the table,
but I was trying to do it later, which seemed harder to do. I didn't consider
wiping it immediately, but I now realise this data isn't needed afterwards. I
only tested amd64, but I trust it will work for other architectures. Please go
ahead with this.
Powered by blists - more mailing lists