From a149eb4d87453f58418ad32c570090739a3e0dd6 Mon Sep 17 00:00:00 2001 From: Olivier Deprez Date: Tue, 27 Sep 2022 13:34:01 +0200 Subject: [PATCH] fix: backtrace stack unwind misses lr adjustment When pointer authentication is used the frame record return address includes the pointer authentication code hence it must be masked out when willing to compare the pointer value with another address or checking its validity. The stack unwind function missed one case of adjusting the return address leading to a misinterpreted corrupted stack frame error message. Signed-off-by: Olivier Deprez Change-Id: I435140937d5fd0f43da27c77d96056b7606d87e9 --- common/backtrace/backtrace.c | 44 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/common/backtrace/backtrace.c b/common/backtrace/backtrace.c index 25e2c707b..89380b3e4 100644 --- a/common/backtrace/backtrace.c +++ b/common/backtrace/backtrace.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2018-2022, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -37,6 +37,23 @@ struct frame_record { uintptr_t return_addr; }; +static inline uintptr_t extract_address(uintptr_t address) +{ + uintptr_t ret = address; + +#if ENABLE_PAUTH + /* + * When pointer authentication is enabled, the LR value saved on the + * stack contains a PAC. It must be stripped to retrieve the return + * address. + */ + + xpaci(ret); +#endif + + return ret; +} + const char *get_el_str(unsigned int el) { if (el == 3U) { @@ -53,18 +70,11 @@ const char *get_el_str(unsigned int el) * the current EL, false otherwise. */ #ifdef __aarch64__ -static bool is_address_readable(uintptr_t addr) +static bool is_address_readable(uintptr_t address) { unsigned int el = get_current_el(); + uintptr_t addr = extract_address(address); -#if ENABLE_PAUTH - /* - * When pointer authentication is enabled, the LR value saved on the - * stack contains a PAC. It must be stripped to retrieve the return - * address. - */ - xpaci(addr); -#endif if (el == 3U) { ats1e3r(addr); } else if (el == 2U) { @@ -185,7 +195,8 @@ static void unwind_stack(struct frame_record *fr, uintptr_t current_pc, return; } - if (fr->return_addr != link_register) { + call_site = extract_address(fr->return_addr); + if (call_site != link_register) { printf("ERROR: Corrupted stack (frame record address = %p)\n", fr); return; @@ -207,16 +218,9 @@ static void unwind_stack(struct frame_record *fr, uintptr_t current_pc, * call was made is the instruction before the return address, * which is always 4 bytes before it. */ - call_site = fr->return_addr - 4U; -#if ENABLE_PAUTH - /* - * When pointer authentication is enabled, the LR value saved on - * the stack contains a PAC. It must be stripped to retrieve the - * return address. - */ - xpaci(call_site); -#endif + call_site = extract_address(fr->return_addr) - 4U; + /* * If the address is invalid it means that the frame record is * probably corrupted.