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

On Mon, 26 Feb 2024 at 23:24, Heinrich Schuchardt heinrich.schuchardt@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@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@linaro.org