From 7cd4334226fad5c7866027d1da41e5afc1bf9f5e Mon Sep 17 00:00:00 2001 From: Jerome Forissier Date: Mon, 10 Oct 2016 18:17:15 +0200 Subject: [PATCH 1/4] arm32: unwind: fix incorrect return status After the unwind code was imported from FreeBSD sources, it was slightly modified to invert some logic. One return slipped through. Signed-off-by: Jerome Forissier Reviewed-by: Etienne Carriere Reviewed-by: Jens Wiklander --- core/arch/arm/kernel/unwind_arm32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/arch/arm/kernel/unwind_arm32.c b/core/arch/arm/kernel/unwind_arm32.c index fa75e962e38..0473f539f8c 100644 --- a/core/arch/arm/kernel/unwind_arm32.c +++ b/core/arch/arm/kernel/unwind_arm32.c @@ -248,7 +248,7 @@ static bool unwind_exec_insn(struct unwind_state *state) mask = unwind_exec_read_byte(state); if (mask == 0 || (mask & 0xf0) != 0) - return 1; + return false; /* Update SP */ update_vsp = 1; From 9c5e2f875c03f42912abe1e07fd3314a05e39412 Mon Sep 17 00:00:00 2001 From: Jerome Forissier Date: Mon, 10 Oct 2016 18:20:07 +0200 Subject: [PATCH 2/4] arm32: unwind: convert int to bool The return status of unwind_tab() is used as a boolean, so change its type. Signed-off-by: Jerome Forissier Reviewed-by: Etienne Carriere Reviewed-by: Jens Wiklander --- core/arch/arm/kernel/unwind_arm32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/arch/arm/kernel/unwind_arm32.c b/core/arch/arm/kernel/unwind_arm32.c index 0473f539f8c..b8b53c84888 100644 --- a/core/arch/arm/kernel/unwind_arm32.c +++ b/core/arch/arm/kernel/unwind_arm32.c @@ -283,7 +283,7 @@ static bool unwind_exec_insn(struct unwind_state *state) } /* Performs the unwind of a function */ -static int unwind_tab(struct unwind_state *state) +static bool unwind_tab(struct unwind_state *state) { uint32_t entry; From e386996cd4049bdc55c8432ff2cf6e06ca363db2 Mon Sep 17 00:00:00 2001 From: Jerome Forissier Date: Mon, 10 Oct 2016 18:21:17 +0200 Subject: [PATCH 3/4] arm32: unwind: mark tee_svc_do_call() with .cantunwind The assembly function tee_svc_do_call() manipulates the stack pointer but does not use the proper unwind directives when doing so. As a result, the compiler can't generate proper unwind information. This can lead to crashes or infinite loops if unwinding is performed at runtime. Given that there is nothing of much interest below this function, we simply add a .cantundwind directive to stop unwinding here. Signed-off-by: Jerome Forissier Reviewed-by: Etienne Carriere Reviewed-by: Jens Wiklander --- core/arch/arm/tee/arch_svc_a32.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/arch/arm/tee/arch_svc_a32.S b/core/arch/arm/tee/arch_svc_a32.S index ef6a0dfbd1a..d9c725c90f9 100644 --- a/core/arch/arm/tee/arch_svc_a32.S +++ b/core/arch/arm/tee/arch_svc_a32.S @@ -42,8 +42,8 @@ */ FUNC tee_svc_do_call , : UNWIND( .fnstart) +UNWIND( .cantunwind) push {r5-r9, lr} -UNWIND( .save {r5-r9, lr}) mov r7, sp mov r8, r0 mov r9, r1 From 18e8c5336c7e18b0959be9fad78777fe1cc0383b Mon Sep 17 00:00:00 2001 From: Jerome Forissier Date: Mon, 10 Oct 2016 18:25:13 +0200 Subject: [PATCH 4/4] arm32: unwind: print_stack(): fix unwind_state print_stack() must save r7 and r11 in the unwind_state structure. Not doing so will likely result in a crash dunring unwind. Register r7 is typically used as a frame pointer by GCC in Thumb2 mode, while r11 (a.k.a. fp) is the frame pointer in ARM mode. Also, set PC to the beginning of print_stack() since there's no point in going further inside the function. Fixes: https://github.com/OP-TEE/optee_os/issues/1069 Signed-off-by: Jerome Forissier Tested-by: Jerome Forissier (HiKey) Tested-by: Jerome Forissier (QEMU) Reviewed-by: Etienne Carriere Reviewed-by: Jens Wiklander --- core/arch/arm/include/arm32.h | 16 ++++++++++++++++ core/arch/arm/kernel/unwind_arm32.c | 8 ++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/core/arch/arm/include/arm32.h b/core/arch/arm/include/arm32.h index 78887bb6482..9b8ee63d309 100644 --- a/core/arch/arm/include/arm32.h +++ b/core/arch/arm/include/arm32.h @@ -558,6 +558,22 @@ static __always_inline uint32_t read_lr(void) asm volatile ("mov %0, lr" : "=r" (val)); return val; } + +static __always_inline uint32_t read_fp(void) +{ + uint32_t val; + + asm volatile ("mov %0, fp" : "=r" (val)); + return val; +} + +static __always_inline uint32_t read_r7(void) +{ + uint32_t val; + + asm volatile ("mov %0, r7" : "=r" (val)); + return val; +} #endif /*ASM*/ #endif /*ARM32_H*/ diff --git a/core/arch/arm/kernel/unwind_arm32.c b/core/arch/arm/kernel/unwind_arm32.c index b8b53c84888..7efe94ba081 100644 --- a/core/arch/arm/kernel/unwind_arm32.c +++ b/core/arch/arm/kernel/unwind_arm32.c @@ -367,10 +367,14 @@ void print_stack(int level) { struct unwind_state state; - memset(&state, 0, sizeof(state)); + memset(state.registers, 0, sizeof(state.registers)); + /* r7: Thumb-style frame pointer */ + state.registers[7] = read_r7(); + /* r11: ARM-style frame pointer */ + state.registers[FP] = read_fp(); state.registers[SP] = read_sp(); state.registers[LR] = read_lr(); - state.registers[PC] = read_pc(); + state.registers[PC] = (uint32_t)print_stack; do { switch (level) {