From 82ace41da43a54436a2690996c3a4bcf838f4f0e Mon Sep 17 00:00:00 2001 From: Flavio Ceolin Date: Mon, 25 Nov 2024 13:58:40 -0800 Subject: [PATCH] security: Additional option for stack canaries Previously, when stack canaries were enabled, Zephyr applied this protection to all functions. This commit introduces a new option that allows stack canary protection to be applied selectively to specific functions based on certain criteria. Signed-off-by: Flavio Ceolin --- CMakeLists.txt | 2 ++ cmake/compiler/arcmwdt/compiler_flags.cmake | 1 + cmake/compiler/compiler_flags_template.cmake | 1 + cmake/compiler/gcc/compiler_flags.cmake | 3 ++ kernel/CMakeLists.txt | 2 +- kernel/Kconfig | 38 +++++++++++++++----- kernel/compiler_stack_protect.c | 2 +- kernel/init.c | 8 ++--- kernel/xip.c | 8 ++--- 9 files changed, 47 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 505eeafff46..cf86d1ac8a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -175,6 +175,8 @@ endif() # @Intent: Set compiler flags to detect general stack overflows across all functions if(CONFIG_STACK_CANARIES) zephyr_compile_options($) +elseif(CONFIG_STACK_CANARIES_STRONG) + zephyr_compile_options($) endif() # @Intent: Obtain compiler optimizations flags and store in variables diff --git a/cmake/compiler/arcmwdt/compiler_flags.cmake b/cmake/compiler/arcmwdt/compiler_flags.cmake index 6b334c1fe84..3f8a46f4f01 100644 --- a/cmake/compiler/arcmwdt/compiler_flags.cmake +++ b/cmake/compiler/arcmwdt/compiler_flags.cmake @@ -168,6 +168,7 @@ set_compiler_property(PROPERTY imacros -imacros) # Security canaries. #no support of -mstack-protector-guard=global" set_compiler_property(PROPERTY security_canaries -fstack-protector-all) +set_compiler_property(PROPERTY security_canaries_strong -fstack-protector-strong) #no support of _FORTIFY_SOURCE" set_compiler_property(PROPERTY security_fortify_compile_time) diff --git a/cmake/compiler/compiler_flags_template.cmake b/cmake/compiler/compiler_flags_template.cmake index 8c540c15d06..f2d03b3d86a 100644 --- a/cmake/compiler/compiler_flags_template.cmake +++ b/cmake/compiler/compiler_flags_template.cmake @@ -92,6 +92,7 @@ set_compiler_property(PROPERTY coverage) # Security canaries flags. set_compiler_property(PROPERTY security_canaries) +set_compiler_property(PROPERTY security_canaries_strong) set_compiler_property(PROPERTY security_fortify_compile_time) set_compiler_property(PROPERTY security_fortify_run_time) diff --git a/cmake/compiler/gcc/compiler_flags.cmake b/cmake/compiler/gcc/compiler_flags.cmake index ae1bfe45976..83d58e4896c 100644 --- a/cmake/compiler/gcc/compiler_flags.cmake +++ b/cmake/compiler/gcc/compiler_flags.cmake @@ -168,12 +168,15 @@ set_compiler_property(PROPERTY coverage -fprofile-arcs -ftest-coverage -fno-inli # Security canaries. set_compiler_property(PROPERTY security_canaries -fstack-protector-all) +set_compiler_property(PROPERTY security_canaries_strong -fstack-protector-strong) # Only a valid option with GCC 7.x and above, so let's do check and set. if(CONFIG_STACK_CANARIES_TLS) check_set_compiler_property(APPEND PROPERTY security_canaries -mstack-protector-guard=tls) + check_set_compiler_property(APPEND PROPERTY security_canaries_strong -mstack-protector-guard=tls) else() check_set_compiler_property(APPEND PROPERTY security_canaries -mstack-protector-guard=global) + check_set_compiler_property(APPEND PROPERTY security_canaries_global -mstack-protector-guard=global) endif() diff --git a/kernel/CMakeLists.txt b/kernel/CMakeLists.txt index 8477508b98a..e5db39713b6 100644 --- a/kernel/CMakeLists.txt +++ b/kernel/CMakeLists.txt @@ -140,7 +140,7 @@ set_target_properties( __ZEPHYR_SUPERVISOR__ ) -target_sources_ifdef(CONFIG_STACK_CANARIES kernel PRIVATE compiler_stack_protect.c) +target_sources_ifdef(CONFIG_REQUIRES_STACK_CANARIES kernel PRIVATE compiler_stack_protect.c) target_sources_ifdef(CONFIG_SYS_CLOCK_EXISTS kernel PRIVATE timeout.c timer.c) target_sources_ifdef(CONFIG_ATOMIC_OPERATIONS_C kernel PRIVATE atomic_c.c) target_sources_ifdef(CONFIG_MMU kernel PRIVATE mmu.c) diff --git a/kernel/Kconfig b/kernel/Kconfig index 361fb67f17e..407e091c35d 100644 --- a/kernel/Kconfig +++ b/kernel/Kconfig @@ -869,25 +869,47 @@ config XIP menu "Security Options" -config STACK_CANARIES - bool "Compiler stack canaries" - depends on ENTROPY_GENERATOR || TEST_RANDOM_GENERATOR - select NEED_LIBC_MEM_PARTITION if !STACK_CANARIES_TLS +config REQUIRES_STACK_CANARIES + bool help - This option enables compiler stack canaries. + Hidden option to signal that software stack protection is required. +choice + prompt "Stack canaries protection options" + optional + help If stack canaries are supported by the compiler, it will emit extra code that inserts a canary value into the stack frame when a function is entered and validates this value upon exit. Stack corruption (such as that caused by buffer overflow) results in a fatal error condition for the running entity. - Enabling this option can result in a significant increase - in footprint and an associated decrease in performance. + Enabling this option, depending on the level chosen, can result in a + significant increase in footprint and a corresponding decrease in performance. If stack canaries are not supported by the compiler an error will occur at build time. -if STACK_CANARIES +config STACK_CANARIES + bool "Maximum protection available" + depends on ENTROPY_GENERATOR || TEST_RANDOM_GENERATOR + select NEED_LIBC_MEM_PARTITION if !STACK_CANARIES_TLS + select REQUIRES_STACK_CANARIES + help + This option enables compiler stack canaries for all functions. + +config STACK_CANARIES_STRONG + bool "Strong protection" + depends on ENTROPY_GENERATOR || TEST_RANDOM_GENERATOR + select NEED_LIBC_MEM_PARTITION if !STACK_CANARIES_TLS + select REQUIRES_STACK_CANARIES + help + This option enables compiler stack canaries in functions that call alloca, + functions that have local array definitiion or have references to local + frame addresses. + +endchoice + +if REQUIRES_STACK_CANARIES config STACK_CANARIES_TLS bool "Stack canaries using thread local storage" diff --git a/kernel/compiler_stack_protect.c b/kernel/compiler_stack_protect.c index e8408a33ed1..166f43e00b4 100644 --- a/kernel/compiler_stack_protect.c +++ b/kernel/compiler_stack_protect.c @@ -10,7 +10,7 @@ * * This module provides functions to support compiler stack protection * using canaries. This feature is enabled with configuration - * CONFIG_STACK_CANARIES=y. + * CONFIG_STACK_CANARIES=y or CONFIG_STACK_CANARIES_STRONG=y. * * When this feature is enabled, the compiler generated code refers to * function __stack_chk_fail and global variable __stack_chk_guard. diff --git a/kernel/init.c b/kernel/init.c index a43f040ee7f..d6c4fb3c70c 100644 --- a/kernel/init.c +++ b/kernel/init.c @@ -289,13 +289,13 @@ void z_bss_zero_pinned(void) } #endif /* CONFIG_LINKER_USE_PINNED_SECTION */ -#ifdef CONFIG_STACK_CANARIES +#ifdef CONFIG_REQUIRES_STACK_CANARIES #ifdef CONFIG_STACK_CANARIES_TLS extern Z_THREAD_LOCAL volatile uintptr_t __stack_chk_guard; #else extern volatile uintptr_t __stack_chk_guard; #endif /* CONFIG_STACK_CANARIES_TLS */ -#endif /* CONFIG_STACK_CANARIES */ +#endif /* CONFIG_REQUIRES_STACK_CANARIES */ /* LCOV_EXCL_STOP */ @@ -778,13 +778,13 @@ FUNC_NORETURN void z_cstart(void) #endif z_sys_init_run_level(INIT_LEVEL_PRE_KERNEL_2); -#ifdef CONFIG_STACK_CANARIES +#ifdef CONFIG_REQUIRES_STACK_CANARIES uintptr_t stack_guard; z_early_rand_get((uint8_t *)&stack_guard, sizeof(stack_guard)); __stack_chk_guard = stack_guard; __stack_chk_guard <<= 8; -#endif /* CONFIG_STACK_CANARIES */ +#endif /* CONFIG_REQUIRES_STACK_CANARIES */ #ifdef CONFIG_TIMING_FUNCTIONS_NEED_AT_BOOT timing_init(); diff --git a/kernel/xip.c b/kernel/xip.c index 898dfe2ae4c..66ce7907dde 100644 --- a/kernel/xip.c +++ b/kernel/xip.c @@ -10,13 +10,13 @@ #include #include -#ifdef CONFIG_STACK_CANARIES +#ifdef CONFIG_REQUIRES_STACK_CANARIES #ifdef CONFIG_STACK_CANARIES_TLS extern Z_THREAD_LOCAL volatile uintptr_t __stack_chk_guard; #else extern volatile uintptr_t __stack_chk_guard; #endif /* CONFIG_STACK_CANARIES_TLS */ -#endif /* CONFIG_STACK_CANARIES */ +#endif /* CONFIG_REQUIRES_STACK_CANARIES */ /** * @brief Copy the data section from ROM to RAM @@ -49,7 +49,7 @@ void z_data_copy(void) data_copy_xip_relocation(); #endif /* CONFIG_CODE_DATA_RELOCATION */ #ifdef CONFIG_USERSPACE -#ifdef CONFIG_STACK_CANARIES +#ifdef CONFIG_REQUIRES_STACK_CANARIES /* stack canary checking is active for all C functions. * __stack_chk_guard is some uninitialized value living in the * app shared memory sections. Preserve it, and don't make any @@ -70,6 +70,6 @@ void z_data_copy(void) #else z_early_memcpy(&_app_smem_start, &_app_smem_rom_start, _app_smem_end - _app_smem_start); -#endif /* CONFIG_STACK_CANARIES */ +#endif /* CONFIG_REQUIRES_STACK_CANARIES */ #endif /* CONFIG_USERSPACE */ }