feat(lib/stack_protector): Add stack protector option
This patch adds `-fstack-protector-strong` as a USER
build option to TF-RMM.
Fixes #35
Signed-off-by: Jacob Man Chun Yiu <jacobmanchun.yiu@arm.com>
Change-Id: I5cd9898a4433d445b7a701288040f7b7476f28d3
diff --git a/app/common/el0_app/CMakeLists.txt b/app/common/el0_app/CMakeLists.txt
index 91c08ec..d725dfc 100644
--- a/app/common/el0_app/CMakeLists.txt
+++ b/app/common/el0_app/CMakeLists.txt
@@ -10,6 +10,7 @@
rmm-lib-console
rmm-lib-common
rmm-lib-debug
+ rmm-lib-stack_protector
)
target_include_directories(rmm-app-common-el0_app
diff --git a/app/common/el0_app/src/aarch64/el0_app_helpers.S b/app/common/el0_app/src/aarch64/el0_app_helpers.S
index 0ee6c45..4153a1c 100644
--- a/app/common/el0_app/src/aarch64/el0_app_helpers.S
+++ b/app/common/el0_app/src/aarch64/el0_app_helpers.S
@@ -21,6 +21,9 @@
mov x23, x3
mov x24, x4
mov x25, lr
+#ifdef STACK_PROTECTOR_ENABLED
+ bl rmm_init_stack_canary
+#endif /* STACK_PROTECTOR_ENABLED */
bl init_console
mov x0, x20
mov x1, x21
diff --git a/cmake/CommonConfigs.cmake b/cmake/CommonConfigs.cmake
index 89d4c27..44a3fb2 100644
--- a/cmake/CommonConfigs.cmake
+++ b/cmake/CommonConfigs.cmake
@@ -91,6 +91,17 @@
ADVANCED)
#
+# Enable the Stack protection compiler flag.
+# Having the PAUTH and BTI feature enabled makes the software-based
+# stack frame canary redundant. Enabling the software canary could
+# have a performance degradation. Hence the default is OFF.
+#
+arm_config_option(
+ NAME STACK_PROTECTOR
+ HELP "Enable Stack Protection Compiler Flags"
+ string OFF)
+
+#
# Introduce a pseudo-library purely for applying flags to RMM's libraries.
# This is applied to any targets created after this point.
#
@@ -138,6 +149,14 @@
target_compile_definitions(rmm-common
INTERFACE "RMM_NUM_PAGES_PER_STACK=UL(${RMM_NUM_PAGES_PER_STACK})")
+# Set stack protector option.
+if(STACK_PROTECTOR)
+ target_compile_definitions(rmm-common
+ INTERFACE "STACK_PROTECTOR_ENABLED=1")
+ message(STATUS "Stack Protector is Enabled.")
+ add_compile_options(-fstack-protector-strong)
+endif()
+
if(RMM_FPU_USE_AT_REL2 AND RMM_ARCH STREQUAL aarch64)
target_compile_definitions(rmm-common
INTERFACE "RMM_FPU_USE_AT_REL2=1")
diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 7c434ad..0fa4a77 100644
--- a/docs/getting_started/build-options.rst
+++ b/docs/getting_started/build-options.rst
@@ -297,6 +297,7 @@
ATTEST_PLAT_TOKEN_SIZE , ,0x1000 ,"Maximum size in bytes expected for the Attestation platform token"
PLAT_ARM_MAX_MEM_BANKS , ,2 ,"Maximum possible number of DRAM and COH/NCOH device memory banks allowed in Arm platform layer"
ATTEST_EL3_TOKEN_SIGN ,ON|OFF ,OFF ,"Use EL3 service to sign realm attestation token."
+ STACK_PROTECTOR ,ON | OFF ,OFF ,"Enable the stack protector compiler option."
.. _llvm_build:
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index eaf615b..114496c 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -16,6 +16,7 @@
rmm-lib-smc
rmm-lib-s2tt
rmm-lib-slot_buf
+ rmm-lib-stack_protector
rmm-lib-xlat)
add_subdirectory("allocator")
@@ -30,6 +31,7 @@
add_subdirectory("smc")
add_subdirectory("s2tt")
add_subdirectory("slot_buf")
+add_subdirectory("stack_protector")
add_subdirectory("xlat")
# Device Assignment feature in RMM v1.1 needs rmm-lib-spdm_requester
diff --git a/lib/arch/include/aarch64/entropy.h b/lib/arch/include/aarch64/entropy.h
index 70885e0..9bff229 100644
--- a/lib/arch/include/aarch64/entropy.h
+++ b/lib/arch/include/aarch64/entropy.h
@@ -12,7 +12,10 @@
/*
* Write 8 bytes of random data in random. Returns true on success, false on
* failure.
+ * One of its use case is to initialize canary value. Hence the
+ * `no_stack_protector` attribute.
*/
+__attribute__((no_stack_protector))
static inline bool arch_collect_entropy(uint64_t *random)
{
unsigned long rc;
diff --git a/lib/arch/include/aarch64/instr_helpers.h b/lib/arch/include/aarch64/instr_helpers.h
index 7cdd2e8..0e0d2c5 100644
--- a/lib/arch/include/aarch64/instr_helpers.h
+++ b/lib/arch/include/aarch64/instr_helpers.h
@@ -13,7 +13,9 @@
* registers
*********************************************************************/
+/* Force inline */
#define DEFINE_SYSREG_READ_FUNC_(_name, _reg_name) \
+__attribute__((__always_inline__)) \
static inline u_register_t read_ ## _name(void) \
{ \
u_register_t v; \
@@ -22,6 +24,7 @@
}
#define DEFINE_SYSREG_WRITE_FUNC_(_name, _reg_name) \
+__attribute__((__always_inline__)) \
static inline void write_ ## _name(u_register_t v) \
{ \
(void)v; /* To avoid MISRA-C:2012-2.7 warnings */ \
@@ -43,6 +46,7 @@
/* Define function for simple system instruction */
#define DEFINE_SYSOP_FUNC(_op) \
+__attribute__((__always_inline__)) \
static inline void (_op)(void) \
{ \
__asm__ (#_op); \
@@ -50,6 +54,7 @@
/* Define function for system instruction with register parameter */
#define DEFINE_SYSOP_PARAM_FUNC(_op) \
+__attribute__((__always_inline__)) \
static inline void (_op)(uint64_t v) \
{ \
(void)v; /* To avoid MISRA-C:2012-2.7 warnings */ \
@@ -58,6 +63,7 @@
/* Define function for system instruction with type specifier */
#define DEFINE_SYSOP_TYPE_FUNC(_op, _type) \
+__attribute__((__always_inline__)) \
static inline void (_op ## _type)(void) \
{ \
__asm__ (#_op " " #_type : : : "memory"); \
@@ -65,6 +71,7 @@
/* Define function for system instruction with register parameter */
#define DEFINE_SYSOP_TYPE_PARAM_FUNC(_op, _type) \
+__attribute__((__always_inline__)) \
static inline void (_op ## _type)(uint64_t v) \
{ \
(void)v; /* To avoid MISRA-C:2012-2.7 warnings */ \
diff --git a/lib/arch/include/aarch64/spinlock.h b/lib/arch/include/aarch64/spinlock.h
index 9d75d96..0c07b88 100644
--- a/lib/arch/include/aarch64/spinlock.h
+++ b/lib/arch/include/aarch64/spinlock.h
@@ -15,6 +15,7 @@
unsigned int val;
} spinlock_t;
+__attribute__((__always_inline__))
static inline void spinlock_acquire(spinlock_t *l)
{
unsigned int tmp;
@@ -38,6 +39,7 @@
);
}
+__attribute__((__always_inline__))
static inline void spinlock_release(spinlock_t *l)
{
/* To avoid misra-c2012-2.7 warnings */
@@ -56,6 +58,7 @@
unsigned char val;
} byte_spinlock_t;
+__attribute__((__always_inline__))
static inline void byte_spinlock_acquire(byte_spinlock_t *l)
{
unsigned int tmp;
@@ -79,6 +82,7 @@
);
}
+__attribute__((__always_inline__))
static inline void byte_spinlock_release(byte_spinlock_t *l)
{
/* To avoid misra-c2012-2.7 warnings */
diff --git a/lib/stack_protector/CMakeLists.txt b/lib/stack_protector/CMakeLists.txt
new file mode 100644
index 0000000..b7f76d9
--- /dev/null
+++ b/lib/stack_protector/CMakeLists.txt
@@ -0,0 +1,18 @@
+#
+# SPDX-License-Identifier: BSD-3-Clause
+# SPDX-FileCopyrightText: Copyright TF-RMM Contributors.
+#
+
+add_library(rmm-lib-stack_protector)
+
+target_include_directories(rmm-lib-stack_protector
+ PUBLIC "include"
+ PRIVATE "src")
+
+target_link_libraries(rmm-lib-stack_protector
+ PRIVATE rmm-lib-common
+ rmm-lib-debug
+ rmm-lib-arch)
+
+
+target_sources(rmm-lib-stack_protector PRIVATE "src/stack_protector.c")
diff --git a/lib/stack_protector/include/stack_protector.h b/lib/stack_protector/include/stack_protector.h
new file mode 100644
index 0000000..0e4d2a9
--- /dev/null
+++ b/lib/stack_protector/include/stack_protector.h
@@ -0,0 +1,19 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * SPDX-FileCopyrightText: Copyright TF-RMM Contributors.
+ */
+
+#ifndef STACK_PROTECTOR_H
+#define STACK_PROTECTOR_H
+
+#include <types.h>
+
+/* Function called when stack protection check fails. */
+void __dead2 __stack_chk_fail(void);
+
+/*
+ * Initialize or update the canary value.
+ */
+void rmm_init_stack_canary(void);
+
+#endif /* STACK_PROTECTOR_H */
diff --git a/lib/stack_protector/src/stack_protector.c b/lib/stack_protector/src/stack_protector.c
new file mode 100644
index 0000000..ba5603d
--- /dev/null
+++ b/lib/stack_protector/src/stack_protector.c
@@ -0,0 +1,63 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * SPDX-FileCopyrightText: Copyright TF-RMM Contributors.
+ */
+
+#include <cpuid.h>
+#include <debug.h>
+#include <entropy.h>
+#include <spinlock.h>
+#include <stack_protector.h>
+#include <types.h>
+#include <utils_def.h>
+
+static spinlock_t stack_canary_init_lock = {0U};
+
+static bool stack_init;
+
+/* __stack_chk_guard and __stack_chk_fail is only used in AArch64.
+ *
+ * NOT used in fake host because there are already protection mechanisms
+ * which interfere with this mechanism.
+ */
+
+/*
+ * Canary value used by the compiler runtime checks to detect stack corruption.
+ */
+/* cppcheck-suppress [misra-c2012-8.4] */
+/* coverity[misra_c_2012_rule_8_4_violation:SUPPRESS] */
+u_register_t __stack_chk_guard = (u_register_t) 1;
+
+/*
+ * Function called when the stack canary check fails.
+ * It must not return and the program will stop.
+ */
+void __dead2 __stack_chk_fail(void)
+{
+ ERROR("Stack corruption detected\n");
+ panic();
+}
+
+/*
+ * Function is used to initialize/update canary value.
+ * no_stack protector attribute needs to be specified
+ * for this function.
+ */
+__attribute__((no_stack_protector))
+void rmm_init_stack_canary(void)
+{
+ spinlock_acquire(&stack_canary_init_lock);
+
+ /* If initialized already, then skip*/
+ if (stack_init) {
+ spinlock_release(&stack_canary_init_lock);
+ return;
+ }
+
+ while (!(arch_collect_entropy(&__stack_chk_guard))) {
+ }
+
+ stack_init = true;
+
+ spinlock_release(&stack_canary_init_lock);
+}
diff --git a/runtime/core/aarch64/head.S b/runtime/core/aarch64/head.S
index 44fe582..2d5bbc8 100644
--- a/runtime/core/aarch64/head.S
+++ b/runtime/core/aarch64/head.S
@@ -159,9 +159,18 @@
* xlat tables.
*/
bl plat_setup
+
bl xlat_enable_mmu_el2
bl pauth_init_enable_el2
+#ifdef STACK_PROTECTOR_ENABLED
+ /*
+ * The canary init function uses spin_lock and hence it is
+ * called after MMU is enabled.
+ */
+ bl rmm_init_stack_canary
+#endif /* STACK_PROTECTOR_ENABLED */
+
/*
* Clear granules memory
*/
diff --git a/tools/shrinkwrap/configs/rmm-stack_protector.yaml b/tools/shrinkwrap/configs/rmm-stack_protector.yaml
new file mode 100644
index 0000000..731e479
--- /dev/null
+++ b/tools/shrinkwrap/configs/rmm-stack_protector.yaml
@@ -0,0 +1,22 @@
+#
+# SPDX-License-Identifier: BSD-3-Clause
+# SPDX-FileCopyrightText: Copyright TF-RMM Contributors.
+#
+
+%YAML 1.2
+---
+description: >-
+ Overlay to enable stack protection compilation on shrinkwrap
+
+buildex:
+ btvars:
+ # Stack_Protector compiler flag
+ STACK_PROTECTOR:
+ type: "ON"
+ value: string
+
+build:
+ rmm:
+ sourcedir: ${btvar:RMM_SRC}
+ params:
+ -DSTACK_PROTECTOR: ${btvar:STACK_PROTECTOR}