From 5d1c104f9aa7e1f52607679db96e5695cac266e7 Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux Date: Fri, 8 Jul 2016 14:37:40 +0100 Subject: [PATCH] Introduce SEPARATE_CODE_AND_RODATA build flag At the moment, all BL images share a similar memory layout: they start with their code section, followed by their read-only data section. The two sections are contiguous in memory. Therefore, the end of the code section and the beginning of the read-only data one might share a memory page. This forces both to be mapped with the same memory attributes. As the code needs to be executable, this means that the read-only data stored on the same memory page as the code are executable as well. This could potentially be exploited as part of a security attack. This patch introduces a new build flag called SEPARATE_CODE_AND_RODATA, which isolates the code and read-only data on separate memory pages. This in turn allows independent control of the access permissions for the code and read-only data. This has an impact on memory footprint, as padding bytes need to be introduced between the code and read-only data to ensure the segragation of the two. To limit the memory cost, the memory layout of the read-only section has been changed in this case. - When SEPARATE_CODE_AND_RODATA=0, the layout is unchanged, i.e. the read-only section still looks like this (padding omitted): | ... | +-------------------+ | Exception vectors | +-------------------+ | Read-only data | +-------------------+ | Code | +-------------------+ BLx_BASE In this case, the linker script provides the limits of the whole read-only section. - When SEPARATE_CODE_AND_RODATA=1, the exception vectors and read-only data are swapped, such that the code and exception vectors are contiguous, followed by the read-only data. This gives the following new layout (padding omitted): | ... | +-------------------+ | Read-only data | +-------------------+ | Exception vectors | +-------------------+ | Code | +-------------------+ BLx_BASE In this case, the linker script now exports 2 sets of addresses instead: the limits of the code and the limits of the read-only data. Refer to the Firmware Design guide for more details. This provides platform code with a finer-grained view of the image layout and allows it to map these 2 regions with the appropriate access permissions. Note that SEPARATE_CODE_AND_RODATA applies to all BL images. Change-Id: I936cf80164f6b66b6ad52b8edacadc532c935a49 --- Makefile | 6 ++++++ bl1/bl1.ld.S | 38 ++++++++++++++++++++++++++++++++++ bl2/bl2.ld.S | 25 +++++++++++++++++++++++ bl2u/bl2u.ld.S | 18 ++++++++++++++++ bl31/bl31.ld.S | 42 ++++++++++++++++++++++++++++++++++++++ bl32/tsp/tsp.ld.S | 18 ++++++++++++++++ docs/firmware-design.md | 8 +++++++- include/common/bl_common.h | 8 ++++++++ 8 files changed, 162 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5f4a93c94..800312c94 100644 --- a/Makefile +++ b/Makefile @@ -108,6 +108,10 @@ PL011_GENERIC_UART := 0 ENABLE_PMF := 0 # Flag to enable PSCI STATs functionality ENABLE_PSCI_STAT := 0 +# Whether code and read-only data should be put on separate memory pages. +# The platform Makefile is free to override this value. +SEPARATE_CODE_AND_RODATA := 0 + ################################################################################ # Checkpatch script options @@ -419,6 +423,7 @@ $(eval $(call assert_boolean,SPIN_ON_BL1_EXIT)) $(eval $(call assert_boolean,PL011_GENERIC_UART)) $(eval $(call assert_boolean,ENABLE_PMF)) $(eval $(call assert_boolean,ENABLE_PSCI_STAT)) +$(eval $(call assert_boolean,SEPARATE_CODE_AND_RODATA)) ################################################################################ @@ -448,6 +453,7 @@ $(eval $(call add_define,SPIN_ON_BL1_EXIT)) $(eval $(call add_define,PL011_GENERIC_UART)) $(eval $(call add_define,ENABLE_PMF)) $(eval $(call add_define,ENABLE_PSCI_STAT)) +$(eval $(call add_define,SEPARATE_CODE_AND_RODATA)) # Define the EL3_PAYLOAD_BASE flag only if it is provided. ifdef EL3_PAYLOAD_BASE $(eval $(call add_define,EL3_PAYLOAD_BASE)) diff --git a/bl1/bl1.ld.S b/bl1/bl1.ld.S index be36b4ee6..b9554d15c 100644 --- a/bl1/bl1.ld.S +++ b/bl1/bl1.ld.S @@ -45,6 +45,43 @@ SECTIONS ASSERT(. == ALIGN(4096), "BL1_RO_BASE address is not aligned on a page boundary.") +#if SEPARATE_CODE_AND_RODATA + .text . : { + __TEXT_START__ = .; + *bl1_entrypoint.o(.text*) + *(.text*) + *(.vectors) + . = NEXT(4096); + __TEXT_END__ = .; + } >ROM + + .rodata . : { + __RODATA_START__ = .; + *(.rodata*) + + /* Ensure 8-byte alignment for descriptors and ensure inclusion */ + . = ALIGN(8); + __PARSER_LIB_DESCS_START__ = .; + KEEP(*(.img_parser_lib_descs)) + __PARSER_LIB_DESCS_END__ = .; + + /* + * Ensure 8-byte alignment for cpu_ops so that its fields are also + * aligned. Also ensure cpu_ops inclusion. + */ + . = ALIGN(8); + __CPU_OPS_START__ = .; + KEEP(*(cpu_ops)) + __CPU_OPS_END__ = .; + + /* + * No need to pad out the .rodata section to a page boundary. Next is + * the .data section, which can mapped in ROM with the same memory + * attributes as the .rodata section. + */ + __RODATA_END__ = .; + } >ROM +#else ro . : { __RO_START__ = .; *bl1_entrypoint.o(.text*) @@ -69,6 +106,7 @@ SECTIONS *(.vectors) __RO_END__ = .; } >ROM +#endif ASSERT(__CPU_OPS_END__ > __CPU_OPS_START__, "cpu_ops not defined for this platform.") diff --git a/bl2/bl2.ld.S b/bl2/bl2.ld.S index a660bda63..fa694de28 100644 --- a/bl2/bl2.ld.S +++ b/bl2/bl2.ld.S @@ -45,6 +45,30 @@ SECTIONS ASSERT(. == ALIGN(4096), "BL2_BASE address is not aligned on a page boundary.") +#if SEPARATE_CODE_AND_RODATA + .text . : { + __TEXT_START__ = .; + *bl2_entrypoint.o(.text*) + *(.text*) + *(.vectors) + . = NEXT(4096); + __TEXT_END__ = .; + } >RAM + + .rodata . : { + __RODATA_START__ = .; + *(.rodata*) + + /* Ensure 8-byte alignment for descriptors and ensure inclusion */ + . = ALIGN(8); + __PARSER_LIB_DESCS_START__ = .; + KEEP(*(.img_parser_lib_descs)) + __PARSER_LIB_DESCS_END__ = .; + + . = NEXT(4096); + __RODATA_END__ = .; + } >RAM +#else ro . : { __RO_START__ = .; *bl2_entrypoint.o(.text*) @@ -67,6 +91,7 @@ SECTIONS . = NEXT(4096); __RO_END__ = .; } >RAM +#endif /* * Define a linker symbol to mark start of the RW memory area for this diff --git a/bl2u/bl2u.ld.S b/bl2u/bl2u.ld.S index ec1207791..d72589fca 100644 --- a/bl2u/bl2u.ld.S +++ b/bl2u/bl2u.ld.S @@ -45,6 +45,23 @@ SECTIONS ASSERT(. == ALIGN(4096), "BL2U_BASE address is not aligned on a page boundary.") +#if SEPARATE_CODE_AND_RODATA + .text . : { + __TEXT_START__ = .; + *bl2u_entrypoint.o(.text*) + *(.text*) + *(.vectors) + . = NEXT(4096); + __TEXT_END__ = .; + } >RAM + + .rodata . : { + __RODATA_START__ = .; + *(.rodata*) + . = NEXT(4096); + __RODATA_END__ = .; + } >RAM +#else ro . : { __RO_START__ = .; *bl2u_entrypoint.o(.text*) @@ -61,6 +78,7 @@ SECTIONS . = NEXT(4096); __RO_END__ = .; } >RAM +#endif /* * Define a linker symbol to mark start of the RW memory area for this diff --git a/bl31/bl31.ld.S b/bl31/bl31.ld.S index 33cbe4b79..743e65c40 100644 --- a/bl31/bl31.ld.S +++ b/bl31/bl31.ld.S @@ -46,6 +46,47 @@ SECTIONS ASSERT(. == ALIGN(4096), "BL31_BASE address is not aligned on a page boundary.") +#if SEPARATE_CODE_AND_RODATA + .text . : { + __TEXT_START__ = .; + *bl31_entrypoint.o(.text*) + *(.text*) + *(.vectors) + . = NEXT(4096); + __TEXT_END__ = .; + } >RAM + + .rodata . : { + __RODATA_START__ = .; + *(.rodata*) + + /* Ensure 8-byte alignment for descriptors and ensure inclusion */ + . = ALIGN(8); + __RT_SVC_DESCS_START__ = .; + KEEP(*(rt_svc_descs)) + __RT_SVC_DESCS_END__ = .; + +#if ENABLE_PMF + /* Ensure 8-byte alignment for descriptors and ensure inclusion */ + . = ALIGN(8); + __PMF_SVC_DESCS_START__ = .; + KEEP(*(pmf_svc_descs)) + __PMF_SVC_DESCS_END__ = .; +#endif /* ENABLE_PMF */ + + /* + * Ensure 8-byte alignment for cpu_ops so that its fields are also + * aligned. Also ensure cpu_ops inclusion. + */ + . = ALIGN(8); + __CPU_OPS_START__ = .; + KEEP(*(cpu_ops)) + __CPU_OPS_END__ = .; + + . = NEXT(4096); + __RODATA_END__ = .; + } >RAM +#else ro . : { __RO_START__ = .; *bl31_entrypoint.o(.text*) @@ -85,6 +126,7 @@ SECTIONS . = NEXT(4096); __RO_END__ = .; } >RAM +#endif ASSERT(__CPU_OPS_END__ > __CPU_OPS_START__, "cpu_ops not defined for this platform.") diff --git a/bl32/tsp/tsp.ld.S b/bl32/tsp/tsp.ld.S index 9c7ed3815..7e24f66d7 100644 --- a/bl32/tsp/tsp.ld.S +++ b/bl32/tsp/tsp.ld.S @@ -46,6 +46,23 @@ SECTIONS ASSERT(. == ALIGN(4096), "BL32_BASE address is not aligned on a page boundary.") +#if SEPARATE_CODE_AND_RODATA + .text . : { + __TEXT_START__ = .; + *tsp_entrypoint.o(.text*) + *(.text*) + *(.vectors) + . = NEXT(4096); + __TEXT_END__ = .; + } >RAM + + .rodata . : { + __RODATA_START__ = .; + *(.rodata*) + . = NEXT(4096); + __RODATA_END__ = .; + } >RAM +#else ro . : { __RO_START__ = .; *tsp_entrypoint.o(.text*) @@ -61,6 +78,7 @@ SECTIONS . = NEXT(4096); __RO_END__ = .; } >RAM +#endif /* * Define a linker symbol to mark start of the RW memory area for this diff --git a/docs/firmware-design.md b/docs/firmware-design.md index 575a822aa..b99a2838f 100644 --- a/docs/firmware-design.md +++ b/docs/firmware-design.md @@ -1115,7 +1115,9 @@ All BL images share the following requirements: * The BSS section must be zero-initialised before executing any C code. * The coherent memory section (if enabled) must be zero-initialised as well. * The MMU setup code needs to know the extents of the coherent and read-only - memory regions to set the right memory attributes. + memory regions to set the right memory attributes. When + `SEPARATE_CODE_AND_RODATA=1`, it needs to know more specifically how the + read-only memory region is divided between code and data. The following linker symbols are defined for this purpose: @@ -1126,6 +1128,10 @@ The following linker symbols are defined for this purpose: * `__COHERENT_RAM_UNALIGNED_SIZE__` * `__RO_START__` * `__RO_END__` +* `__TEXT_START__` +* `__TEXT_END__` +* `__RODATA_START__` +* `__RODATA_END__` #### BL1's linker symbols diff --git a/include/common/bl_common.h b/include/common/bl_common.h index c43ad5ef9..646a81724 100644 --- a/include/common/bl_common.h +++ b/include/common/bl_common.h @@ -143,8 +143,16 @@ * Declarations of linker defined symbols to help determine memory layout of * BL images */ +#if SEPARATE_CODE_AND_RODATA +extern unsigned long __TEXT_START__; +extern unsigned long __TEXT_END__; +extern unsigned long __RODATA_START__; +extern unsigned long __RODATA_END__; +#else extern unsigned long __RO_START__; extern unsigned long __RO_END__; +#endif + #if IMAGE_BL2 extern unsigned long __BL2_END__; #elif IMAGE_BL2U