kernel: canaries: Allow using TLS to store it

Add new option to use thread local storage for stack
canaries. This makes harder to find the canaries location
and value. This is made optional because there is
a performance and size penalty when using it.

Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>

arch/Kconfig

@@ -92,6 +92,7 @@ config X86
 	select NEED_LIBC_MEM_PARTITION if USERSPACE && TIMING_FUNCTIONS \
 					  && !BOARD_HAS_TIMING_FUNCTIONS \
 					  && !SOC_HAS_TIMING_FUNCTIONS
+	select ARCH_HAS_STACK_CANARIES_TLS
 	help
 	  x86 architecture
 
@@ -600,6 +601,9 @@ config ARCH_HAS_SUSPEND_TO_RAM
 	help
 	  When selected, the architecture supports suspend-to-RAM (S2RAM).
 
+config ARCH_HAS_STACK_CANARIES_TLS
+	bool
+
 #
 # Other architecture related options
 #

cmake/compiler/gcc/compiler_flags.cmake

@@ -160,7 +160,12 @@ set_compiler_property(PROPERTY coverage -fprofile-arcs -ftest-coverage -fno-inli
 set_compiler_property(PROPERTY security_canaries -fstack-protector-all)
 
 # 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)
+else()
   check_set_compiler_property(APPEND PROPERTY security_canaries -mstack-protector-guard=global)
+endif()
+
 
 if(NOT CONFIG_NO_OPTIMIZATIONS)
   # _FORTIFY_SOURCE: Detect common-case buffer overflows for certain functions

include/zephyr/sys/libc-hooks.h

@@ -92,7 +92,8 @@ __syscall size_t zephyr_fwrite(const void *ZRESTRICT ptr, size_t size,
 extern struct k_mem_partition z_malloc_partition;
 #endif
 
-#if defined(CONFIG_NEWLIB_LIBC) || defined(CONFIG_STACK_CANARIES) || \
+#if defined(CONFIG_NEWLIB_LIBC) || (defined(CONFIG_STACK_CANARIES) && \
+			    !defined(CONFIG_STACK_CANARIES_TLS)) || \
 defined(CONFIG_PICOLIBC) || defined(CONFIG_NEED_LIBC_MEM_PARTITION)
 /* - All newlib globals will be placed into z_libc_partition.
  * - Minimal C library globals, if any, will be placed into

kernel/Kconfig

@@ -836,6 +836,24 @@ config STACK_CANARIES
 	  If stack canaries are not supported by the compiler an error
 	  will occur at build time.
 
+if STACK_CANARIES
+
+config STACK_CANARIES_TLS
+	bool "Stack canaries using thread local storage"
+	depends on THREAD_LOCAL_STORAGE
+	depends on ARCH_HAS_STACK_CANARIES_TLS
+	help
+	  This option enables compiler stack canaries on TLS.
+
+	  Stack canaries will leave in the thread local storage and
+	  each thread will have its own canary. This makes harder
+	  to predict the canary location and value.
+
+	  When enabled this causes an additional performance penalty
+	  during thread creations because it needs a new random value
+	  per thread.
+endif
+
 config EXECUTE_XOR_WRITE
 	bool "W^X for memory partitions"
 	depends on USERSPACE

kernel/compiler_stack_protect.c

@@ -46,7 +46,9 @@ void _StackCheckHandler(void)
  * Symbol referenced by GCC compiler generated code for canary value.
  * The canary value gets initialized in z_cstart().
  */
-#ifdef CONFIG_USERSPACE
+#ifdef CONFIG_STACK_CANARIES_TLS
+__thread uintptr_t __stack_chk_guard;
+#elif CONFIG_USERSPACE
 K_APP_DMEM(z_libc_partition) uintptr_t __stack_chk_guard;
 #else
 __noinit uintptr_t __stack_chk_guard;

kernel/init.c

@@ -217,7 +217,11 @@ void z_bss_zero_pinned(void)
 #endif /* CONFIG_LINKER_USE_PINNED_SECTION */
 
 #ifdef CONFIG_STACK_CANARIES
+#ifdef CONFIG_STACK_CANARIES_TLS
+extern __thread volatile uintptr_t __stack_chk_guard;
+#else
 extern volatile uintptr_t __stack_chk_guard;
+#endif
 #endif /* CONFIG_STACK_CANARIES */
 
 /* LCOV_EXCL_STOP */

kernel/xip.c

@@ -11,7 +11,11 @@
 #include <zephyr/linker/linker-defs.h>
 
 #ifdef CONFIG_STACK_CANARIES
+#ifdef CONFIG_STACK_CANARIES_TLS
+extern __thread volatile uintptr_t __stack_chk_guard;
+#else
 extern volatile uintptr_t __stack_chk_guard;
+#endif /* CONFIG_STACK_CANARIES_TLS */
 #endif /* CONFIG_STACK_CANARIES */
 
 /**

lib/os/thread_entry.c

@@ -12,11 +12,16 @@
  */
 
 #include <zephyr/kernel.h>
-
 #ifdef CONFIG_THREAD_LOCAL_STORAGE
+#include <zephyr/random/rand32.h>
+
 __thread k_tid_t z_tls_current;
 #endif
 
+#ifdef CONFIG_STACK_CANARIES_TLS
+extern __thread volatile uintptr_t __stack_chk_guard;
+#endif /* CONFIG_STACK_CANARIES_TLS */
+
 /*
  * Common thread entry point function (used by all threads)
  *
@@ -33,6 +38,13 @@ FUNC_NORETURN void z_thread_entry(k_thread_entry_t entry,
 #ifdef CONFIG_THREAD_LOCAL_STORAGE
 	z_tls_current = z_current_get();
 #endif
+#ifdef CONFIG_STACK_CANARIES_TLS
+	uintptr_t stack_guard;
+
+	sys_rand_get((uint8_t *)&stack_guard, sizeof(stack_guard));
+	__stack_chk_guard = stack_guard;
+	__stack_chk_guard <<= 8;
+#endif	/* CONFIG_STACK_CANARIES */
 	entry(p1, p2, p3);
 
 	k_thread_abort(k_current_get());