From d435238dc364f0c9f0e41661365f83d83899829d Mon Sep 17 00:00:00 2001 From: Manish Pandey Date: Tue, 11 Oct 2022 17:28:14 +0100 Subject: [PATCH 1/2] fix(bl31): harden check in delegate_async_ea Following hardening done around ESR_EL3 register usage - Panic if exception is anyting other than SError - AET bit is only valid if DFSC is 0x11, move DFSC check before AET. Signed-off-by: Manish Pandey Change-Id: Ib15159920f6cad964332fd40f88943aee2bc73b4 --- bl31/aarch64/ea_delegate.S | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/bl31/aarch64/ea_delegate.S b/bl31/aarch64/ea_delegate.S index 5e53ab4b6..dbb32344d 100644 --- a/bl31/aarch64/ea_delegate.S +++ b/bl31/aarch64/ea_delegate.S @@ -195,23 +195,30 @@ endfunc delegate_sync_ea */ func delegate_async_ea #if RAS_EXTENSION + /* Check Exception Class to ensure SError, as this function should + * only be invoked for SError. If that is not the case, which implies + * either an HW error or programming error, panic. + */ + ubfx x2, x1, #ESR_EC_SHIFT, #ESR_EC_LENGTH + cmp x2, EC_SERROR + b.ne do_panic /* * Check for Implementation Defined Syndrome. If so, skip checking * Uncontainable error type from the syndrome as the format is unknown. */ tbnz x1, #SERROR_IDS_BIT, 1f + /* AET only valid when DFSC is 0x11 */ + ubfx x2, x1, #EABORT_DFSC_SHIFT, #EABORT_DFSC_WIDTH + cmp x2, #DFSC_SERROR + b.ne 1f + /* * Check for Uncontainable error type. If so, route to the platform * fatal error handler rather than the generic EA one. */ - ubfx x2, x1, #EABORT_AET_SHIFT, #EABORT_AET_WIDTH - cmp x2, #ERROR_STATUS_UET_UC - b.ne 1f - - /* Check DFSC for SError type */ - ubfx x3, x1, #EABORT_DFSC_SHIFT, #EABORT_DFSC_WIDTH - cmp x3, #DFSC_SERROR + ubfx x3, x1, #EABORT_AET_SHIFT, #EABORT_AET_WIDTH + cmp x3, #ERROR_STATUS_UET_UC b.ne 1f no_ret plat_handle_uncontainable_ea From 0ae4a3a3f0cd841b83f2944dde9837ea67f08813 Mon Sep 17 00:00:00 2001 From: Manish Pandey Date: Tue, 1 Nov 2022 16:16:55 +0000 Subject: [PATCH 2/2] fix(debug): decouple "get_el_str()" from backtrace get_el_str() was implemented under ENABLE_BACKTRACE macro but being used at generic places too, this causes multiple definition of this function. Remove duplicate definition of this function and move it out of backtrace scope. Also, this patch fixes a small bug where in default case S-EL1 is returned which ideally should be EL1, as there is no notion of security state in EL string. Signed-off-by: Manish Pandey Change-Id: Ib186ea03b776e2478eff556065449ebd478c3538 --- common/backtrace/backtrace.c | 11 ----------- include/common/debug.h | 3 ++- plat/common/aarch64/plat_common.c | 6 ++---- plat/marvell/armada/a3k/common/a3700_ea.c | 12 ------------ 4 files changed, 4 insertions(+), 28 deletions(-) diff --git a/common/backtrace/backtrace.c b/common/backtrace/backtrace.c index 89380b3e4..f994ae5b6 100644 --- a/common/backtrace/backtrace.c +++ b/common/backtrace/backtrace.c @@ -54,17 +54,6 @@ static inline uintptr_t extract_address(uintptr_t address) return ret; } -const char *get_el_str(unsigned int el) -{ - if (el == 3U) { - return "EL3"; - } else if (el == 2U) { - return "EL2"; - } else { - return "S-EL1"; - } -} - /* * Returns true if the address points to a virtual address that can be read at * the current EL, false otherwise. diff --git a/include/common/debug.h b/include/common/debug.h index a7ca0d788..af47999c3 100644 --- a/include/common/debug.h +++ b/include/common/debug.h @@ -91,9 +91,10 @@ # define VERBOSE(...) no_tf_log(LOG_MARKER_VERBOSE __VA_ARGS__) #endif +const char *get_el_str(unsigned int el); + #if ENABLE_BACKTRACE void backtrace(const char *cookie); -const char *get_el_str(unsigned int el); #else #define backtrace(x) #endif diff --git a/plat/common/aarch64/plat_common.c b/plat/common/aarch64/plat_common.c index 8f998af3f..851ed24fb 100644 --- a/plat/common/aarch64/plat_common.c +++ b/plat/common/aarch64/plat_common.c @@ -67,17 +67,15 @@ int plat_sdei_validate_entry_point(uintptr_t ep, unsigned int client_mode) } #endif -#if !ENABLE_BACKTRACE -static const char *get_el_str(unsigned int el) +const char *get_el_str(unsigned int el) { if (el == MODE_EL3) { return "EL3"; } else if (el == MODE_EL2) { return "EL2"; } - return "S-EL1"; + return "EL1"; } -#endif /* !ENABLE_BACKTRACE */ /* RAS functions common to AArch64 ARM platforms */ void plat_default_ea_handler(unsigned int ea_reason, uint64_t syndrome, void *cookie, diff --git a/plat/marvell/armada/a3k/common/a3700_ea.c b/plat/marvell/armada/a3k/common/a3700_ea.c index bc12845a5..fd4e3b247 100644 --- a/plat/marvell/armada/a3k/common/a3700_ea.c +++ b/plat/marvell/armada/a3k/common/a3700_ea.c @@ -16,18 +16,6 @@ #define A53_SERR_INT_AXI_SLVERR_ON_EXTERNAL_ACCESS 0xbf000002 -#if !ENABLE_BACKTRACE -static const char *get_el_str(unsigned int el) -{ - if (el == MODE_EL3) { - return "EL3"; - } else if (el == MODE_EL2) { - return "EL2"; - } - return "S-EL1"; -} -#endif /* !ENABLE_BACKTRACE */ - /* * This source file with custom plat_ea_handler function is compiled only when * building TF-A with compile option HANDLE_EA_EL3_FIRST=1