[PATCH v2 2/2] arm: separate .data and .text sections of EFI binaries

[Ilias Apalodimas]

On Mon, 26 Feb 2024 at 23:24, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> EFI binaries should not contain sections that are both writable and
> executable. Separate the RX .text section from the RW .data section.
>
> We currently don't created relocation sections (.rel.*) for our EFI
> binaries. Anyway these would have to be converted to PE/COFF relocations.
> Enumerate them under DISCARD and add a comment.
>
> Correct the characteristics of the sections.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> v2:
>         Consider that 32-bit arm uses .rel and not .rela relocations
>         and discard them as they are cannot be used in EFI binaries.
>         Correct the characteristics of the .reloc section.
> ---
>  arch/arm/lib/crt0_arm_efi.S  | 40 ++++++++++++++++++++++++++++--------
>  arch/arm/lib/elf_arm_efi.lds | 28 ++++++++++++++++++-------
>  2 files changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S
> index 7a4e5dff75..7219c0f2fe 100644
> --- a/arch/arm/lib/crt0_arm_efi.S
> +++ b/arch/arm/lib/crt0_arm_efi.S
> @@ -23,7 +23,7 @@ pe_header:
>         .long   IMAGE_NT_SIGNATURE              /* 'PE' */
>  coff_header:
>         .short  IMAGE_FILE_MACHINE_THUMB        /* Mixed ARM/Thumb */
> -       .short  2                               /* nr_sections */
> +       .short  3                               /* nr_sections */
>         .long   0                               /* TimeDateStamp */
>         .long   0                               /* PointerToSymbolTable */
>         .long   0                               /* NumberOfSymbols */
> @@ -98,22 +98,44 @@ section_table:
>         .long   0                       /* PointerToLineNumbers */
>         .short  0                       /* NumberOfRelocations */
>         .short  0                       /* NumberOfLineNumbers */
> -       .long   0x42100040              /* Characteristics (section flags) */
> +       /* Characteristics (section flags) */
> +       .long   (IMAGE_SCN_MEM_READ | \
> +                IMAGE_SCN_MEM_DISCARDABLE | \
> +                IMAGE_SCN_CNT_INITIALIZED_DATA)
>
>         .ascii  ".text"
>         .byte   0
>         .byte   0
>         .byte   0                       /* end of 0 padding of section name */
> -       .long   _edata - _start         /* VirtualSize */
> +       .long   _text_size              /* VirtualSize */
>         .long   _start - image_base     /* VirtualAddress */
> -       .long   _edata - _start         /* SizeOfRawData */
> +       .long   _text_size              /* SizeOfRawData */
>         .long   _start - image_base     /* PointerToRawData */
> +       .long   0                       /* PointerToRelocations */
> +       .long   0                       /* PointerToLineNumbers */
> +       .short  0                       /* NumberOfRelocations */
> +       .short  0                       /* NumberOfLineNumbers */
> +       /* Characteristics (section flags) */
> +       .long   (IMAGE_SCN_MEM_READ | \
> +                IMAGE_SCN_MEM_EXECUTE | \
> +                IMAGE_SCN_CNT_CODE)
>
> -       .long   0               /* PointerToRelocations (0 for executables) */
> -       .long   0               /* PointerToLineNumbers (0 for executables) */
> -       .short  0               /* NumberOfRelocations  (0 for executables) */
> -       .short  0               /* NumberOfLineNumbers  (0 for executables) */
> -       .long   0xe0500020      /* Characteristics (section flags) */
> +       .ascii  ".data"
> +       .byte   0
> +       .byte   0
> +       .byte   0                       /* end of 0 padding of section name */
> +       .long   _data_size              /* VirtualSize */
> +       .long   _data - image_base      /* VirtualAddress */
> +       .long   _data_size              /* SizeOfRawData */
> +       .long   _data - image_base      /* PointerToRawData */
> +       .long   0                       /* PointerToRelocations */
> +       .long   0                       /* PointerToLineNumbers */
> +       .short  0                       /* NumberOfRelocations */
> +       .short  0                       /* NumberOfLineNumbers */
> +       /* Characteristics (section flags) */
> +       .long   (IMAGE_SCN_MEM_WRITE | \
> +                IMAGE_SCN_MEM_READ | \
> +                IMAGE_SCN_CNT_INITIALIZED_DATA)
>
>         .align          12
>  _start:
> diff --git a/arch/arm/lib/elf_arm_efi.lds b/arch/arm/lib/elf_arm_efi.lds
> index 767ebda635..41440594aa 100644
> --- a/arch/arm/lib/elf_arm_efi.lds
> +++ b/arch/arm/lib/elf_arm_efi.lds
> @@ -7,6 +7,12 @@
>
>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>  OUTPUT_ARCH(arm)
> +
> +PHDRS
> +{
> +       data PT_LOAD FLAGS(3); /* PF_W | PF_X */
> +}
> +
>  ENTRY(_start)
>  SECTIONS
>  {
> @@ -18,11 +24,13 @@ SECTIONS
>                 *(.gnu.linkonce.t.*)
>                 *(.srodata)
>                 *(.rodata*)
> +               . = ALIGN(16);
> +               *(.dynamic);
>                 . = ALIGN(512);
>         }
>         _etext = .;
>         _text_size = . - _text;
> -       .dynamic  : { *(.dynamic) }
> +       . = ALIGN(4096);
>         .data : {
>                 _data = .;
>                 *(.sdata)
> @@ -47,14 +55,20 @@ SECTIONS
>                 . = ALIGN(512);
>                 _bss_end = .;
>                 _edata = .;
> -       }
> -       .rel.dyn : { *(.rel.dyn) }
> -       .rel.plt : { *(.rel.plt) }
> -       .rel.got : { *(.rel.got) }
> -       .rel.data : { *(.rel.data) *(.rel.data*) }
> -       _data_size = . - _etext;
> +       } :data
> +       _data_size = . - _data;
>
>         /DISCARD/ : {
> +               /*
> +                * We don't support relocations. These would have to be
> +                * translated from ELF to PE format and added to the .reloc
> +                * section.
> +                */
> +               *(.rel.dyn)
> +               *(.rel.plt)
> +               *(.rel.got)
> +               *(.rel.data)
> +               *(.rel.data*)
>                 *(.rel.reloc)
>                 *(.eh_frame)
>                 *(.note.GNU-stack)
> --
> 2.43.0
>

Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>