[PATCH 1/1] arm: separate .data and .text sections of EFI binaries
EFI binaries should not contain sections that are both writable and executable. Separate the RX .text section from the RW .data section.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- arch/arm/lib/crt0_arm_efi.S | 35 +++++++++++++++++++++++++++-------- arch/arm/lib/elf_arm_efi.lds | 33 ++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index f6deca9a14..ba9fada500 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 */ @@ -107,16 +107,35 @@ section_table: .byte 0 .byte 0 .byte 0 /* end of 0 padding of section name */ - .long _edata - _start /* VirtualSize */ + .long _etext - _start /* VirtualSize */ .long _start - image_base /* VirtualAddress */ - .long _edata - _start /* SizeOfRawData */ + .long _etext - _start /* 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..75341df5dd 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); /* SHF_WRITE | SHF_ALLOC */ +} + ENTRY(_start) SECTIONS { @@ -18,11 +24,17 @@ SECTIONS *(.gnu.linkonce.t.*) *(.srodata) *(.rodata*) + . = ALIGN(16); + *(.dynamic); . = ALIGN(512); } + .rela.dyn : { *(.rela.dyn) } + .rela.plt : { *(.rela.plt) } + .rela.got : { *(.rela.got) } + .rela.data : { *(.rela.data) *(.rela.data*) } _etext = .; _text_size = . - _text; - .dynamic : { *(.dynamic) } + . = ALIGN(4096); .data : { _data = .; *(.sdata) @@ -47,20 +59,19 @@ 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 = _edata - _data;
+ . = ALIGN(4096); + .dynsym : { *(.dynsym) } + . = ALIGN(4096); + .dynstr : { *(.dynstr) } + . = ALIGN(4096); + .note.gnu.build-id : { *(.note.gnu.build-id) } /DISCARD/ : { *(.rel.reloc) *(.eh_frame) *(.note.GNU-stack) - *(.dynsym) - *(.dynstr) - *(.note.gnu.build-id) - *(.comment) } + .comment 0 : { *(.comment) } }
Hi Heinrich,
[...]
}
.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }
Why are we switching from Rel to Rela?
_etext = .; _text_size = . - _text;
.dynamic : { *(.dynamic) }
. = ALIGN(4096); .data : { _data = .; *(.sdata)@@ -47,20 +59,19 @@ 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 = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) } /DISCARD/ : { *(.rel.reloc) *(.eh_frame) *(.note.GNU-stack)
*(.dynsym)*(.dynstr)*(.note.gnu.build-id)*(.comment) }
.comment 0 : { *(.comment) }}
2.43.0
Cheers /Ilias
On 2/16/24 11:35, Ilias Apalodimas wrote:
Hi Heinrich,
[...]
}
.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }Why are we switching from Rel to Rela?
This was wrong.
Looking at the u-boot ELF binary we see these relocation sections:
arm: .rel.dyn arm64: .rela.dyn riscv64: .rela.dyn
In our EFI object files we currently have no relocations. This is why tests did not complain.
In arch/arm/lib/reloc_arm_efi.c we handle DT_REL. In arch/arm/lib/reloc_aarch64_efi.c we handle DT_RELA. In arch/riscv/lib/reloc_riscv_efi.c we handle DT_RELA.
Best regards
Heinrich
_etext = .; _text_size = . - _text;
.dynamic : { *(.dynamic) }
. = ALIGN(4096); .data : { _data = .; *(.sdata)@@ -47,20 +59,19 @@ 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 = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) } /DISCARD/ : { *(.rel.reloc) *(.eh_frame) *(.note.GNU-stack)
*(.dynsym)*(.dynstr)*(.note.gnu.build-id)*(.comment) }
.comment 0 : { *(.comment) }-- 2.43.0
Cheers /Ilias
On 2/24/24 10:16, Heinrich Schuchardt wrote:
On 2/16/24 11:35, Ilias Apalodimas wrote:
Hi Heinrich,
[...]
} + .rela.dyn : { *(.rela.dyn) } + .rela.plt : { *(.rela.plt) } + .rela.got : { *(.rela.got) } + .rela.data : { *(.rela.data) *(.rela.data*) }
Why are we switching from Rel to Rela?
This was wrong.
Looking at the u-boot ELF binary we see these relocation sections:
arm: .rel.dyn arm64: .rela.dyn riscv64: .rela.dyn
In our EFI object files we currently have no relocations. This is why tests did not complain.
In arch/arm/lib/reloc_arm_efi.c we handle DT_REL. In arch/arm/lib/reloc_aarch64_efi.c we handle DT_RELA. In arch/riscv/lib/reloc_riscv_efi.c we handle DT_RELA.
To be truthful our generated binaries are only usable because we have no relocations.
If we had relocations, copying these to the .text or .data section would would not work. We would have to translate them to the relocation format of PE files and put them into the .reloc section like EDK II does in BaseTools/Source/C/GenFw/Elf32Convert.c.
Best regards
Heinrich
Best regards
Heinrich
_etext = .; _text_size = . - _text; - .dynamic : { *(.dynamic) } + . = ALIGN(4096); .data : { _data = .; *(.sdata) @@ -47,20 +59,19 @@ 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 = _edata - _data;
+ . = ALIGN(4096); + .dynsym : { *(.dynsym) } + . = ALIGN(4096); + .dynstr : { *(.dynstr) } + . = ALIGN(4096); + .note.gnu.build-id : { *(.note.gnu.build-id) } /DISCARD/ : { *(.rel.reloc) *(.eh_frame) *(.note.GNU-stack) - *(.dynsym) - *(.dynstr) - *(.note.gnu.build-id) - *(.comment) } + .comment 0 : { *(.comment) } } -- 2.43.0
Cheers /Ilias
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas