[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 17:53:55 +0300
On Thu, Feb 2, 2017 at 5:48 PM, Vladimir 'phcoder' Serbinenko
> 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>
>> > 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.
OK, I see.