On Mon, Jun 15, 2020 at 11:56:05AM -0400, Arvind Sankar wrote: > On Mon, Jun 15, 2020 at 11:58:43AM +0200, Ard Biesheuvel wrote: > > 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. > > > > This was based on the runtime-relocation removing patch series (see > cover letter). > https://lore.kernel.org/linux-efi/20200526170226.2371024-1-nivedita@xxxxxxxxxxxx/ > > I can rework it to apply on mainline if we decide this patch could be > useful. > > > 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?) > > > > The EFI stub code itself doesn't use much stack. The largest frame is > 720 bytes and the rest are below 300, so it probably doesn't even reach > 4k. The risk is really that inside the firmware it uses stack space more > liberally given it can assume it has 128KiB available. A safer > alternative would be to switch to the firmware stack only when actually > calling the firmware, inside the mixed-mode thunk. So one thing that mostly mitigates this is that the boot heap, which at this time would be unused, is right below the stack and is at least 64KiB in size. Taking that into account we really have 80KiB of stack available, so this might be fragile wrt future changes but right now it should be safe to run on the boot stack. > > Also, this patch fixed up one other small issue, which is that when we > enter via the compat 32-bit entry, we will call efi_pe_entry with a > misaligned stack (0 mod 16 instead of 8 mod 16). It gets correctly > aligned once efi_pe_entry finishes and calls efi_stub_entry though, so > most of the stub will still execute with proper alignment. Should I do a patch just for the alignment thing then?
