On Tue, 16 Jun 2020 at 20:48, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > 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? Yes please. I am about to send some fixes to -tip so I'd like to include it. The other issue can be dealt with for the next merge window, and we can do some light testing first.
