Re: [PATCH] i386/relocator: Align stack in grub_relocator64_efi relocator

[Andrei Borzenkov]

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.

> +LOCAL(skip_efi_stack_align):
> +#endif
>         /* mov imm64, %rax */
>         .byte   0x48
>         .byte   0xb8
> @@ -128,8 +136,10 @@ LOCAL(jump_addr):
>  VARIABLE(grub_relocator64_rip)
>         .quad   0
>
> +#ifdef GRUB_MACHINE_EFI
>         /* Here grub_relocator64_efi_start() ends. Ufff... */
>  VARIABLE(grub_relocator64_efi_end)
> +#endif
>
>  #ifndef __x86_64__
>         .p2align        4
> --
> 1.7.10.4
>