On Tue, 26 May 2020 at 19:02, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > The UEFI specification requires a 128KiB stack during boot services. On > a native mode boot, the EFI stub executes on the firmware stack. > However, on a mixed-mode boot, startup_32 switches to the kernel's boot > stack, which is only 16KiB, and the EFI stub is executed with this > stack. > > To avoid any potential problems with running out of stack space, save > and restore the UEFI stack pointer in the mixed-mode entry, so that the > EFI stub can use the firmware stack in this case as well. > > Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx> This does not apply onto v5.8-rc1, and I was going to take it as a fix. However, are we sure this is safe? Do we have a ballpark figure of how much stack we use in the stub? This is one of those things I am reluctant to change, given that we are not sure that firmware implementations conform to this, and IA32 firmware was not designed to boot a 64-bit image (which might use more stack space?) > --- > arch/x86/boot/compressed/head_64.S | 46 ++++++++++++++++++++---------- > 1 file changed, 31 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S > index 4b7ad1dfbea6..923e5c381804 100644 > --- a/arch/x86/boot/compressed/head_64.S > +++ b/arch/x86/boot/compressed/head_64.S > @@ -237,21 +237,7 @@ SYM_FUNC_START(startup_32) > * used to perform that far jump. > */ > pushl $__KERNEL_CS > - leal rva(startup_64)(%ebp), %eax > -#ifdef CONFIG_EFI_MIXED > - movl rva(efi32_boot_args)(%ebp), %edi > - cmp $0, %edi > - jz 1f > - leal rva(efi64_stub_entry)(%ebp), %eax > - movl rva(efi32_boot_args+4)(%ebp), %esi > - movl rva(efi32_boot_args+8)(%ebp), %edx // saved bootparams pointer > - cmpl $0, %edx > - jnz 1f > - leal rva(efi_pe_entry)(%ebp), %eax > - movl %edi, %ecx // MS calling convention > - movl %esi, %edx > -1: > -#endif > + leal rva(.L64bit)(%ebp), %eax > pushl %eax > > /* Enter paged protected Mode, activating Long Mode */ > @@ -260,6 +246,31 @@ SYM_FUNC_START(startup_32) > > /* Jump from 32bit compatibility mode into 64bit mode. */ > lret > + > + .code64 > +SYM_INNER_LABEL(.L64bit, SYM_L_LOCAL) > +#ifndef CONFIG_EFI_MIXED > + jmp startup_64 > +#else > + movl efi32_boot_args(%rip), %edi > + testl %edi, %edi > + jz startup_64 > + > + /* Restore firmware stack pointer */ > + movl efi32_boot_sp(%rip), %esp > + /* and align it to 8 mod 16 */ > + andl $~0xf, %esp > + subl $8, %esp > + > + movl efi32_boot_args+4(%rip), %esi > + movl efi32_boot_args+8(%rip), %edx // saved bootparams pointer > + testl %edx, %edx > + jnz efi64_stub_entry > + movl %edi, %ecx // MS calling convention > + movl %esi, %edx > + jmp efi_pe_entry > +#endif > + .code32 > SYM_FUNC_END(startup_32) > > #ifdef CONFIG_EFI_MIXED > @@ -285,6 +296,9 @@ SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL) > movw %cs, rva(efi32_boot_cs)(%ebp) > movw %ds, rva(efi32_boot_ds)(%ebp) > > + /* Save firmware stack pointer */ > + movl %esp, rva(efi32_boot_sp)(%ebp) > + > /* Disable paging */ > movl %cr0, %eax > btrl $X86_CR0_PG_BIT, %eax > @@ -648,6 +662,7 @@ SYM_DATA(image_offset, .long 0) > > #ifdef CONFIG_EFI_MIXED > SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0, 0) > +SYM_DATA_LOCAL(efi32_boot_sp, .long 0) > SYM_DATA(efi_is64, .byte 1) > > #define ST32_boottime 60 // offsetof(efi_system_table_32_t, boottime) > @@ -710,6 +725,7 @@ SYM_FUNC_START(efi32_pe_entry) > movl 12(%ebp), %edx // sys_table > movl -4(%ebp), %esi // loaded_image > movl LI32_image_base(%esi), %esi // loaded_image->image_base > + leave // clear stack frame > movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry > /* > * We need to set the image_offset variable here since startup_32() will > -- > 2.26.2 >
