Re: [PATCH v3 3/4] mkimage: arm64-efi: Align header to page granularity

[Daniel Kiper]

On Mon, Jan 14, 2019 at 04:27:17PM +0100, Alexander Graf wrote:
> In order to enforce NX semantics on non-code pages, UEFI firmware
> may require that all code is EFI_PAGE_SIZE (4k) aligned. A similar
> change has recently been applied to edk2 to accomodate for the same
> fact:
>
>   https://lists.01.org/pipermail/edk2-devel/2018-December/033708.html
>
> This patch adapts grub to also implement the same alignment guarantees
> and thus ensures we can boot even when strict permission checks are in
> place.
>
> Signed-off-by: Alexander Graf <address@hidden>
>
> ---
>
> v1 -> v2:
>
>   - Mention only NX requirement in patch description
>   - Use GRUB_EFI_PAGE_SIZE
>
> v2 -> v3:
>
>   - Apply alignment to all architectures
> ---
>  util/mkimage.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/util/mkimage.c b/util/mkimage.c
> index a670db456..6b372cba5 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -39,6 +39,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <assert.h>
> +#include <grub/efi/memory.h>
>  #include <grub/efi/pe32.h>
>  #include <grub/uboot/image.h>
>  #include <grub/arm/reloc.h>
> @@ -66,14 +67,14 @@
>                                   + sizeof (struct grub_pe32_coff_header) \
>                                   + sizeof (struct grub_pe32_optional_header) 
> \
>                                   + 4 * sizeof (struct 
> grub_pe32_section_table), \
> -                                 GRUB_PE32_SECTION_ALIGNMENT)
> +                                 GRUB_EFI_PAGE_SIZE)
>
>  #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE                
> \
>                                   + GRUB_PE32_SIGNATURE_SIZE          \
>                                   + sizeof (struct grub_pe32_coff_header) \
>                                   + sizeof (struct grub_pe64_optional_header) 
> \
>                                   + 4 * sizeof (struct 
> grub_pe32_section_table), \
> -                                 GRUB_PE32_SECTION_ALIGNMENT)
> +                                 GRUB_EFI_PAGE_SIZE)

GRUB_PE32_SECTION_ALIGNMENT should be changed to GRUB_PE32_FILE_ALIGNMENT.
However, earlier GRUB_PE32_FILE_ALIGNMENT should be defined as 0x20. Yeah,
I know that this is contrary to the PE/COFF spec. Though everybody uses
that value. Even MS...

Then below in the util/mkimage.c some code should look more or less like that:

        reloc_addr = ALIGN_UP (header_size + core_size,
                               GRUB_PE32_FILE_ALIGNMENT);

        pe_size = ALIGN_UP (reloc_addr + layout.reloc_size,
                            GRUB_PE32_FILE_ALIGNMENT);

...and...

        o->file_alignment = grub_host_to_target32 (GRUB_PE32_FILE_ALIGNMENT);

Last line should be changed at least in two places.

Daniel