From 5dcb279df8c2958835d1269635929714a949e878 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Thu, 11 May 2017 13:29:15 -0700 Subject: [PATCH] debug: add stack sentinel feature This places a sentinel value at the lowest 4 bytes of a stack memory region and checks it at various intervals, including when servicing interrupts or context switching. This is implemented on all arches except ARC, which supports stack bounds checking directly in hardware. Signed-off-by: Andrew Boie --- arch/arm/core/exc_exit.S | 11 +++++++++ arch/arm/core/fatal.c | 2 +- arch/arm/core/swap.S | 21 +++++++++++++---- arch/nios2/core/fatal.c | 5 ++++ arch/nios2/core/irq_manage.c | 4 ++++ arch/nios2/core/swap.S | 15 ++++++------ arch/riscv32/core/fatal.c | 2 +- arch/riscv32/core/isr.S | 10 ++++++-- arch/x86/core/fatal.c | 2 +- arch/x86/core/intstub.S | 4 +++- arch/x86/core/swap.S | 6 +++++ arch/xtensa/core/fatal.c | 2 +- arch/xtensa/core/swap.S | 7 ++++++ arch/xtensa/core/xt_zephyr.S | 7 ++++++ kernel/include/kernel_structs.h | 11 +++++++++ kernel/include/ksched.h | 3 +++ kernel/sched.c | 3 +++ kernel/thread.c | 41 +++++++++++++++++++++++++++++++++ subsys/debug/Kconfig | 25 ++++++++++++++++++++ 19 files changed, 162 insertions(+), 19 deletions(-) diff --git a/arch/arm/core/exc_exit.S b/arch/arm/core/exc_exit.S index 35c1c821875..d408ce47874 100644 --- a/arch/arm/core/exc_exit.S +++ b/arch/arm/core/exc_exit.S @@ -103,4 +103,15 @@ _ExcExitWithGdbStub: _EXIT_EXC: #endif /* CONFIG_PREEMPT_ENABLED */ +#ifdef CONFIG_STACK_SENTINEL + push {lr} + bl _check_stack_sentinel +#if defined(CONFIG_ARMV6_M) + pop {r0} + mov lr, r0 +#else + pop {lr} +#endif /* CONFIG_ARMV6_M */ +#endif /* CONFIG_STACK_SENTINEL */ + bx lr diff --git a/arch/arm/core/fatal.c b/arch/arm/core/fatal.c index edd6ba80502..88d5af35517 100644 --- a/arch/arm/core/fatal.c +++ b/arch/arm/core/fatal.c @@ -50,7 +50,7 @@ void _NanoFatalErrorHandler(unsigned int reason, printk("***** Invalid Exit Software Error! *****\n"); break; -#if defined(CONFIG_STACK_CANARIES) +#if defined(CONFIG_STACK_CANARIES) || defined(CONFIG_STACK_SENTINEL) case _NANO_ERR_STACK_CHK_FAIL: printk("***** Stack Check Fail! *****\n"); break; diff --git a/arch/arm/core/swap.S b/arch/arm/core/swap.S index b0ff7634b78..cb7b2e9eb7e 100644 --- a/arch/arm/core/swap.S +++ b/arch/arm/core/swap.S @@ -47,13 +47,24 @@ GDATA(_kernel) SECTION_FUNC(TEXT, __pendsv) +#if defined (CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH) || \ + defined(CONFIG_STACK_SENTINEL) + /* Register the context switch */ + push {lr} + #ifdef CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH - /* Register the context switch */ - push {lr} - bl _sys_k_event_logger_context_switch - pop {r0} - mov lr, r0 + bl _sys_k_event_logger_context_switch #endif +#ifdef CONFIG_STACK_SENTINEL + bl _check_stack_sentinel +#endif +#if defined(CONFIG_ARMV6_M) + pop {r0} + mov lr, r0 +#else + pop {lr} +#endif /* CONFIG_ARMV6_M */ +#endif /* CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH || CONFIG_STACK_SENTINEL */ /* load _kernel into r1 and current k_thread into r2 */ ldr r1, =_kernel diff --git a/arch/nios2/core/fatal.c b/arch/nios2/core/fatal.c index 51e8d4a42c9..ee9664e18e0 100644 --- a/arch/nios2/core/fatal.c +++ b/arch/nios2/core/fatal.c @@ -73,6 +73,11 @@ FUNC_NORETURN void _NanoFatalErrorHandler(unsigned int reason, printk("***** Kernel Panic! *****\n"); break; +#ifdef CONFIG_STACK_SENTINEL + case _NANO_ERR_STACK_CHK_FAIL: + printk("***** Stack overflow *****\n"); + break; +#endif default: printk("**** Unknown Fatal Error %u! ****\n", reason); break; diff --git a/arch/nios2/core/irq_manage.c b/arch/nios2/core/irq_manage.c index 1e7d13667e0..588cc339ef6 100644 --- a/arch/nios2/core/irq_manage.c +++ b/arch/nios2/core/irq_manage.c @@ -18,6 +18,7 @@ #include #include #include +#include void _irq_spurious(void *unused) { @@ -91,5 +92,8 @@ void _enter_irq(u32_t ipending) } _kernel.nested--; +#ifdef CONFIG_STACK_SENTINEL + _check_stack_sentinel(); +#endif } diff --git a/arch/nios2/core/swap.S b/arch/nios2/core/swap.S index dcc2f0e1776..45e52f7e5af 100644 --- a/arch/nios2/core/swap.S +++ b/arch/nios2/core/swap.S @@ -59,15 +59,16 @@ SECTION_FUNC(exception.other, __swap) #if CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH call _sys_k_event_logger_context_switch - /* Restore caller-saved r10. We could have stuck its value - * onto the stack, but less instructions to just use immediates - */ - movhi r10, %hi(_kernel) - ori r10, r10, %lo(_kernel) -#endif /* CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH */ - +#endif +#ifdef CONFIG_STACK_SENTINEL + call _check_stack_sentinel +#endif +#if defined(CONFIG_STACK_SENTINEL) || \ + defined (CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH) + /* restore caller-saved r10 */ movhi r10, %hi(_kernel) ori r10, r10, %lo(_kernel) +#endif /* get cached thread to run */ ldw r2, _kernel_offset_to_ready_q_cache(r10) diff --git a/arch/riscv32/core/fatal.c b/arch/riscv32/core/fatal.c index 42ecc4c4d11..91cc61d4de0 100644 --- a/arch/riscv32/core/fatal.c +++ b/arch/riscv32/core/fatal.c @@ -69,7 +69,7 @@ FUNC_NORETURN void _NanoFatalErrorHandler(unsigned int reason, printk("***** Invalid Exit Software Error! *****\n"); break; -#if defined(CONFIG_STACK_CANARIES) +#if defined(CONFIG_STACK_CANARIES) || defined(CONFIG_STACK_SENTINEL) case _NANO_ERR_STACK_CHK_FAIL: printk("***** Stack Check Fail! *****\n"); break; diff --git a/arch/riscv32/core/isr.S b/arch/riscv32/core/isr.S index 2274cd32207..70d0ccfeb8d 100644 --- a/arch/riscv32/core/isr.S +++ b/arch/riscv32/core/isr.S @@ -270,6 +270,11 @@ on_thread_stack: addi t2, t2, -1 sw t2, _kernel_offset_to_nested(t1) +#ifdef CONFIG_STACK_SENTINEL + call _check_stack_sentinel + la t1, _kernel +#endif + /* Restore thread stack pointer */ lw t0, 0x00(sp) addi sp, t0, 0 @@ -304,7 +309,9 @@ reschedule: #if CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH call _sys_k_event_logger_context_switch #endif /* CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH */ - +#ifdef CONFIG_STACK_SENTINEL + call _check_stack_sentinel +#endif /* Get reference to _kernel */ la t0, _kernel @@ -367,7 +374,6 @@ no_reschedule: /* Restore context at SOC level */ jal ra, __soc_restore_context #endif /* CONFIG_RISCV_SOC_CONTEXT_SAVE */ - /* Restore caller-saved registers from thread stack */ lw ra, __NANO_ESF_ra_OFFSET(sp) lw gp, __NANO_ESF_gp_OFFSET(sp) diff --git a/arch/x86/core/fatal.c b/arch/x86/core/fatal.c index 11a5cd930d0..6f90efcc336 100644 --- a/arch/x86/core/fatal.c +++ b/arch/x86/core/fatal.c @@ -68,7 +68,7 @@ FUNC_NORETURN void _NanoFatalErrorHandler(unsigned int reason, printk("***** Invalid Exit Software Error! *****\n"); break; -#if defined(CONFIG_STACK_CANARIES) +#if defined(CONFIG_STACK_CANARIES) || defined(CONFIG_STACK_SENTINEL) case _NANO_ERR_STACK_CHK_FAIL: printk("***** Stack Check Fail! *****\n"); break; diff --git a/arch/x86/core/intstub.S b/arch/x86/core/intstub.S index 9c6edbfa28b..5b689e70cce 100644 --- a/arch/x86/core/intstub.S +++ b/arch/x86/core/intstub.S @@ -397,7 +397,9 @@ noReschedule: popl %esp /* pop thread stack pointer */ - +#ifdef CONFIG_STACK_SENTINEL + call _check_stack_sentinel +#endif /* fall through to 'nestedInterrupt' */ diff --git a/arch/x86/core/swap.S b/arch/x86/core/swap.S index 8687ebec49a..f0193755c85 100644 --- a/arch/x86/core/swap.S +++ b/arch/x86/core/swap.S @@ -27,6 +27,9 @@ /* externs */ GDATA(_k_neg_eagain) +#ifdef CONFIG_STACK_SENTINEL + GTEXT(_check_stack_sentinel) +#endif /** * @@ -139,6 +142,9 @@ SECTION_FUNC(TEXT, __swap) #ifdef CONFIG_KERNEL_EVENT_LOGGER_CONTEXT_SWITCH /* Register the context switch */ call _sys_k_event_logger_context_switch +#endif +#ifdef CONFIG_STACK_SENTINEL + call _check_stack_sentinel #endif movl _kernel_offset_to_ready_q_cache(%edi), %eax diff --git a/arch/xtensa/core/fatal.c b/arch/xtensa/core/fatal.c index 6a31b98a9c9..2f785f77f2a 100644 --- a/arch/xtensa/core/fatal.c +++ b/arch/xtensa/core/fatal.c @@ -55,7 +55,7 @@ FUNC_NORETURN void _NanoFatalErrorHandler(unsigned int reason, case _NANO_ERR_INVALID_TASK_EXIT: printk("***** Invalid Exit Software Error! *****\n"); break; -#if defined(CONFIG_STACK_CANARIES) +#if defined(CONFIG_STACK_CANARIES) || defined(CONFIG_STACK_SENTINEL) case _NANO_ERR_STACK_CHK_FAIL: printk("***** Stack Check Fail! *****\n"); break; diff --git a/arch/xtensa/core/swap.S b/arch/xtensa/core/swap.S index 319248c6a9e..07f28fe583f 100644 --- a/arch/xtensa/core/swap.S +++ b/arch/xtensa/core/swap.S @@ -82,6 +82,13 @@ __swap: #else call4 _sys_k_event_logger_context_switch #endif +#endif +#ifdef CONFIG_STACK_SENTINEL +#ifdef __XTENSA_CALL0_ABI__ + call0 _check_stack_sentinel +#else + call4 _check_stack_sentinel +#endif #endif /* _thread := _kernel.ready_q.cache */ l32i a3, a2, KERNEL_OFFSET(ready_q_cache) diff --git a/arch/xtensa/core/xt_zephyr.S b/arch/xtensa/core/xt_zephyr.S index 08653f557f5..69ebfd72d57 100644 --- a/arch/xtensa/core/xt_zephyr.S +++ b/arch/xtensa/core/xt_zephyr.S @@ -68,6 +68,13 @@ _zxt_dispatch: /* Restore CPENABLE from task's co-processor save area. */ l16ui a3, a3, THREAD_OFFSET(cpEnable) /* a3 := cp_state->cpenable */ wsr a3, CPENABLE +#endif +#ifdef CONFIG_STACK_SENTINEL +#ifdef __XTENSA_CALL0_ABI__ + call0 _check_stack_sentinel +#else + call4 _check_stack_sentinel +#endif #endif /* * Interrupt stack frame. diff --git a/kernel/include/kernel_structs.h b/kernel/include/kernel_structs.h index 9caef3d365d..616ee318804 100644 --- a/kernel/include/kernel_structs.h +++ b/kernel/include/kernel_structs.h @@ -45,6 +45,10 @@ /* end - states */ +#ifdef CONFIG_STACK_SENTINEL +/* Magic value in lowest bytes of the stack */ +#define STACK_SENTINEL 0xF0F0F0F0 +#endif /* lowest value of _thread_base.preempt at which a thread is non-preemptible */ #define _NON_PREEMPT_THRESHOLD 0x0080 @@ -154,6 +158,13 @@ static ALWAYS_INLINE void _new_thread_init(struct k_thread *thread, #ifdef CONFIG_INIT_STACKS memset(pStack, 0xaa, stackSize); #endif +#ifdef CONFIG_STACK_SENTINEL + /* Put the stack sentinel at the lowest 4 bytes of the stack area. + * We periodically check that it's still present and kill the thread + * if it isn't. + */ + *((u32_t *)pStack) = STACK_SENTINEL; +#endif /* CONFIG_STACK_SENTINEL */ /* Initialize various struct k_thread members */ _init_thread_base(&thread->base, prio, _THREAD_PRESTART, options); diff --git a/kernel/include/ksched.h b/kernel/include/ksched.h index 2b1bfb51ed1..d544f6416a0 100644 --- a/kernel/include/ksched.h +++ b/kernel/include/ksched.h @@ -31,6 +31,9 @@ extern void _update_time_slice_before_swap(void); extern s32_t _ms_to_ticks(s32_t ms); #endif extern void idle(void *, void *, void *); +#ifdef CONFIG_STACK_SENTINEL +extern void _check_stack_sentinel(void); +#endif /* find which one is the next thread to run */ /* must be called with interrupts locked */ diff --git a/kernel/sched.c b/kernel/sched.c index f05004369a0..bb4d3aad297 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -297,6 +297,9 @@ void k_yield(void) if (_current == _get_next_ready_thread()) { irq_unlock(key); +#ifdef CONFIG_STACK_SENTINEL + _check_stack_sentinel(); +#endif } else { _Swap(key); } diff --git a/kernel/thread.c b/kernel/thread.c index c21b18cad32..aa18bea1ff4 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -133,6 +133,44 @@ void _thread_monitor_exit(struct k_thread *thread) } #endif /* CONFIG_THREAD_MONITOR */ +#ifdef CONFIG_STACK_SENTINEL +/* Check that the stack sentinel is still present + * + * The stack sentinel feature writes a magic value to the lowest 4 bytes of + * the thread's stack when the thread is initialized. This value gets checked + * in a few places: + * + * 1) In k_yield() if the current thread is not swapped out + * 2) In the interrupt code, after the interrupt has been serviced, and + * a decision *not* to call _Swap() has been made. + * 3) In _Swap(), check the sentinel in the outgoing thread + * 4) When a thread returns from its entry function to cooperatively terminate + * + * Items 2 and 3 require support in arch/ code. + * + * If the check fails, the thread will be terminated appropriately through + * the system fatal error handler. + */ +void _check_stack_sentinel(void) +{ + u32_t *stack; + + if (_is_thread_prevented_from_running(_current)) { + /* Filter out threads that are dummy threads or already + * marked for termination (_THREAD_DEAD) + */ + return; + } + + stack = (u32_t *)_current->stack_info.start; + if (*stack != STACK_SENTINEL) { + /* Restore it so further checks don't trigger this same error */ + *stack = STACK_SENTINEL; + _k_except_reason(_NANO_ERR_STACK_CHK_FAIL); + } +} +#endif + /* * Common thread entry point function (used by all threads) * @@ -148,6 +186,9 @@ FUNC_NORETURN void _thread_entry(void (*entry)(void *, void *, void *), { entry(p1, p2, p3); +#ifdef CONFIG_STACK_SENTINEL + _check_stack_sentinel(); +#endif #ifdef CONFIG_MULTITHREADING if (_is_thread_essential()) { _k_except_reason(_NANO_ERR_INVALID_TASK_EXIT); diff --git a/subsys/debug/Kconfig b/subsys/debug/Kconfig index 67b540b1123..27a70a7db41 100644 --- a/subsys/debug/Kconfig +++ b/subsys/debug/Kconfig @@ -23,6 +23,31 @@ config STACK_USAGE Generate an extra file that specifies the maximum amount of stack used, on a per-function basis. +config STACK_SENTINEL + bool "Enable stack sentinel" + select THREAD_STACK_INFO + default n + help + Store a magic value at the lowest addresses of a thread's stack. + Periodically check that this value is still present and kill the + thread gracefully if it isn't. This is currently checked in four + places: + + 1) Upon any context switch for the outgoing thread + 2) Any hardware interrupt that doesn't context switch, the check is + performed for the interrupted thread + 3) When a thread returns from its entry point + 4) When a thread calls k_yield() but doesn't context switch + + This feature doesn't prevent corruption and the system may be + in an unusable state. However, given the bizarre behavior associated + with stack overflows, knowledge that this is happening is very + useful. + + This feature is intended for those systems which lack hardware support + for stack overflow protection, or have insufficient system resources + to use that hardware support. + config PRINTK bool prompt "Send printk() to console"