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

For security it is preferable that memory pages for executable code are not writable. So there should be only RX and RW pages. To enable this sections of EFI binaries must be page aligned. Furthermore .text and .data sections must be separated.
We already made the necessary changes for arm64 and riscv64. This series addresses the arm32 architecture.
v2: New patch to page align EFI binary section. Consider that 32-bit arm uses .rel and not .rela relocations and discard them as they are cannot be used in EFI binaries.
Heinrich Schuchardt (2): arm: page align EFI binary section arm: separate .data and .text sections of EFI binaries
arch/arm/lib/crt0_arm_efi.S | 44 +++++++++++++++++++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 28 +++++++++++++++++------ 2 files changed, 54 insertions(+), 18 deletions(-)

Change the alignment of the relocation code in EFI binaries to match page boundaries.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: new patch --- arch/arm/lib/crt0_arm_efi.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index 75ee37b7d3..7a4e5dff75 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -115,14 +115,14 @@ section_table: .short 0 /* NumberOfLineNumbers (0 for executables) */ .long 0xe0500020 /* Characteristics (section flags) */
- .align 9 + .align 12 _start: stmfd sp!, {r0-r2, lr}
adr r1, .L_DYNAMIC ldr r0, [r1] add r1, r0, r1 - adr r0, image_base + adrl r0, image_base bl _relocate teq r0, #0 bne 0f

On Mon, 26 Feb 2024 at 23:24, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Change the alignment of the relocation code in EFI binaries to match page boundaries.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: new patch
arch/arm/lib/crt0_arm_efi.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index 75ee37b7d3..7a4e5dff75 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -115,14 +115,14 @@ section_table: .short 0 /* NumberOfLineNumbers (0 for executables) */ .long 0xe0500020 /* Characteristics (section flags) */
.align 9
.align 12
_start: stmfd sp!, {r0-r2, lr}
adr r1, .L_DYNAMIC ldr r0, [r1] add r1, r0, r1
adr r0, image_base
adrl r0, image_base bl _relocate teq r0, #0 bne 0f
-- 2.43.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

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)

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
participants (2)
-
Heinrich Schuchardt
-
Ilias Apalodimas