[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] i386/relocator: Align stack in grub_relocator64_efi relocato
Re: [PATCH] i386/relocator: Align stack in grub_relocator64_efi relocator
Thu, 2 Feb 2017 15:55:38 +0100
On Thu, Feb 02, 2017 at 02:48:31PM +0000, Vladimir 'phcoder' Serbinenko wrote:
> On Thu, 2 Feb 2017, 15:43 Andrei Borzenkov <address@hidden> wrote:
> > On Thu, Feb 2, 2017 at 5:27 PM, Daniel Kiper <address@hidden>
> > wrote:
> > > Unified Extensible Firmware Interface Specification, Version 2.6,
> > > section 2.3.4, x64 Platforms, boot services, says among others:
> > > The stack must be 16-byte aligned. So, do it. Otherwise OS may
> > > boot only by chance as it happens right now.
> > >
> > > Signed-off-by: Daniel Kiper <address@hidden>
> > > ---
> > > grub-core/lib/i386/relocator64.S | 16 +++++++++++++---
> > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/grub-core/lib/i386/relocator64.S
> > b/grub-core/lib/i386/relocator64.S
> > > index 75725cf..148f38a 100644
> > > --- a/grub-core/lib/i386/relocator64.S
> > > +++ b/grub-core/lib/i386/relocator64.S
> > > @@ -73,14 +73,22 @@ VARIABLE(grub_relocator64_rsp)
> > >
> > > movq %rax, %rsp
> > >
> > > +#ifdef GRUB_MACHINE_EFI
> > > + jmp LOCAL(skip_efi_stack_align)
> > > +
> > > /*
> > > - * Here is grub_relocator64_efi_start() entry point.
> > > - * Following code is shared between grub_relocator64_efi_start()
> > > + * Here is grub_relocator64_efi_start() entry point. Most of the
> > > + * code below is shared between grub_relocator64_efi_start()
> > > * and grub_relocator64_start().
> > > *
> > > - * Think twice before changing anything below!!!
> > > + * Think twice before changing anything there!!!
> > > */
> > > VARIABLE(grub_relocator64_efi_start)
> > > + /* Align the stack as UEFI spec requires. */
> > > + andq $~15, %rsp
> > > +
> > It sounds like it should be fixed on caller side instead. I mean,
> > caller allocated some memory, we cannot just arbitrary adjust it now.
> > It sounds wrong.
> Unlike normal relocator, EFI relocator didn't use .RSP field but instead
> uses EFI stack, so there is no other place to fix this.
The other option is call instead of jmp below but it seems to me a less