xtensa: check stack frame pointer before dumping registers

Check that the stack frame pointer is valid before dumping
any registers while handling exceptions. If the pointer is
invalid, anything it points to will probably be also be
invalid. Accessing them may result in another access
violation.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>

arch/Kconfig

@@ -134,6 +134,7 @@ config XTENSA
 	select ARCH_HAS_TIMING_FUNCTIONS
 	select ARCH_MEM_DOMAIN_DATA if USERSPACE
 	select ARCH_HAS_DIRECTED_IPIS
+	select THREAD_STACK_INFO
 	help
 	  Xtensa architecture
 

arch/xtensa/core/vector_handlers.c

@@ -22,20 +22,136 @@
 #include <_soc_inthandlers.h>
 #endif
 
+#include <kernel_internal.h>
 #include <xtensa_internal.h>
+#include <xtensa_stack.h>
 
 LOG_MODULE_DECLARE(os, CONFIG_KERNEL_LOG_LEVEL);
 
 extern char xtensa_arch_except_epc[];
 extern char xtensa_arch_kernel_oops_epc[];
 
+bool xtensa_is_outside_stack_bounds(uintptr_t addr, size_t sz, uint32_t ps)
+{
+	uintptr_t start, end;
+	struct k_thread *thread = _current;
+	bool was_in_isr, invalid;
+
+	/* Without userspace, there is no privileged stack so the thread stack
+	 * is the whole stack (minus reserved area). So there is no need to
+	 * check for PS == UINT32_MAX for special treatment.
+	 */
+	ARG_UNUSED(ps);
+
+	/* Since both level 1 interrupts and exceptions go through
+	 * the same interrupt vector, both of them increase the nested
+	 * counter in the CPU struct. The architecture vector handler
+	 * moves execution to the interrupt stack when nested goes from
+	 * zero to one. Afterwards, any nested interrupts/exceptions will
+	 * continue running in interrupt stack. Therefore, only when
+	 * nested > 1, then it was running in the interrupt stack, and
+	 * we should check bounds against the interrupt stack.
+	 */
+	was_in_isr = arch_curr_cpu()->nested > 1;
+
+	if ((thread == NULL) || was_in_isr) {
+		/* We were servicing an interrupt or in early boot environment
+		 * and are supposed to be on the interrupt stack.
+		 */
+		int cpu_id;
+
+#ifdef CONFIG_SMP
+		cpu_id = arch_curr_cpu()->id;
+#else
+		cpu_id = 0;
+#endif
+
+		start = (uintptr_t)K_KERNEL_STACK_BUFFER(z_interrupt_stacks[cpu_id]);
+		end = start + CONFIG_ISR_STACK_SIZE;
+#ifdef CONFIG_USERSPACE
+	} else if (ps == UINT32_MAX) {
+		/* Since the stashed PS is inside struct pointed by frame->ptr_to_bsa,
+		 * we need to verify that both frame and frame->ptr_to_bsa are valid
+		 * pointer within the thread stack. Also without PS, we have no idea
+		 * whether we were in kernel mode (using privileged stack) or user
+		 * mode (normal thread stack). So we need to check the whole stack
+		 * area.
+		 *
+		 * And... we cannot account for reserved area since we have no idea
+		 * which to use: ARCH_KERNEL_STACK_RESERVED or ARCH_THREAD_STACK_RESERVED
+		 * as we don't know whether we were in kernel or user mode.
+		 */
+		start = (uintptr_t)thread->stack_obj;
+		end = Z_STACK_PTR_ALIGN(thread->stack_info.start + thread->stack_info.size);
+	} else if (((ps & PS_RING_MASK) == 0U) &&
+		   ((thread->base.user_options & K_USER) == K_USER)) {
+		/* Check if this is a user thread, and that it was running in
+		 * kernel mode. If so, we must have been doing a syscall, so
+		 * check with privileged stack bounds.
+		 */
+		start = thread->stack_info.start - CONFIG_PRIVILEGED_STACK_SIZE;
+		end = thread->stack_info.start;
+#endif
+	} else {
+		start = thread->stack_info.start;
+		end = Z_STACK_PTR_ALIGN(thread->stack_info.start + thread->stack_info.size);
+	}
+
+	invalid = (addr <= start) || ((addr + sz) >= end);
+
+	return invalid;
+}
+
+bool xtensa_is_frame_pointer_valid(_xtensa_irq_stack_frame_raw_t *frame)
+{
+	_xtensa_irq_bsa_t *bsa;
+
+	/* Check if the pointer to the frame is within stack bounds. If not, there is no
+	 * need to test if the BSA (base save area) pointer is also valid as it is
+	 * possibly invalid.
+	 */
+	if (xtensa_is_outside_stack_bounds((uintptr_t)frame, sizeof(*frame), UINT32_MAX)) {
+		return false;
+	}
+
+	/* Need to test if the BSA area is also within stack bounds. The information
+	 * contained within the BSA is only valid if within stack bounds.
+	 */
+	bsa = frame->ptr_to_bsa;
+	if (xtensa_is_outside_stack_bounds((uintptr_t)bsa, sizeof(*bsa), UINT32_MAX)) {
+		return false;
+	}
+
+#ifdef CONFIG_USERSPACE
+	/* With usespace, we have privileged stack and normal thread stack within
+	 * one stack object. So we need to further test whether the frame pointer
+	 * resides in the correct stack based on kernel/user mode.
+	 */
+	if (xtensa_is_outside_stack_bounds((uintptr_t)frame, sizeof(*frame), bsa->ps)) {
+		return false;
+	}
+#endif
+
+	return true;
+}
+
 void xtensa_dump_stack(const void *stack)
 {
 	_xtensa_irq_stack_frame_raw_t *frame = (void *)stack;
-	_xtensa_irq_bsa_t *bsa = frame->ptr_to_bsa;
+	_xtensa_irq_bsa_t *bsa;
 	uintptr_t num_high_regs;
 	int reg_blks_remaining;
 
+	/* Don't dump stack if the stack pointer is invalid as any frame elements
+	 * obtained via de-referencing the frame pointer are probably also invalid.
+	 * Or worse, cause another access violation.
+	 */
+	if (!xtensa_is_frame_pointer_valid(frame)) {
+		return;
+	}
+
+	bsa = frame->ptr_to_bsa;
+
 	/* Calculate number of high registers. */
 	num_high_regs = (uint8_t *)bsa - (uint8_t *)frame + sizeof(void *);
 	num_high_regs /= sizeof(uintptr_t);
@@ -108,9 +224,6 @@ static void print_fatal_exception(void *print_stack, int cause,
 	uint32_t ps, vaddr;
 	_xtensa_irq_bsa_t *bsa = (void *)*(int **)print_stack;
 
-	ps = bsa->ps;
-	pc = (void *)bsa->pc;
-
 	__asm__ volatile("rsr.excvaddr %0" : "=r"(vaddr));
 
 	if (is_dblexc) {
@@ -122,6 +235,19 @@ static void print_fatal_exception(void *print_stack, int cause,
 	LOG_ERR(" ** CPU %d EXCCAUSE %d (%s)",
 		arch_curr_cpu()->id, cause,
 		xtensa_exccause(cause));
+
+	/* Don't print information if the BSA area is invalid as any elements
+	 * obtained via de-referencing the pointer are probably also invalid.
+	 * Or worse, cause another access violation.
+	 */
+	if (xtensa_is_outside_stack_bounds((uintptr_t)bsa, sizeof(*bsa), UINT32_MAX)) {
+		LOG_ERR(" ** VADDR %p Invalid SP %p", (void *)vaddr, print_stack);
+		return;
+	}
+
+	ps = bsa->ps;
+	pc = (void *)bsa->pc;
+
 	LOG_ERR(" **  PC %p VADDR %p", pc, (void *)vaddr);
 
 	if (is_dblexc) {
@@ -217,7 +343,7 @@ static inline DEF_INT_C_HANDLER(1)
  */
 void *xtensa_excint1_c(void *esf)
 {
-	int cause;
+	int cause, reason;
 	int *interrupted_stack = &((struct arch_esf *)esf)->dummy;
 	_xtensa_irq_bsa_t *bsa = (void *)*(int **)interrupted_stack;
 	bool is_fatal_error = false;
@@ -259,11 +385,17 @@ void *xtensa_excint1_c(void *esf)
 		break;
 #endif /* !CONFIG_USERSPACE */
 	default:
+		reason = K_ERR_CPU_EXCEPTION;
+
+		/* If the BSA area is invalid, we cannot trust anything coming out of it. */
+		if (xtensa_is_outside_stack_bounds((uintptr_t)bsa, sizeof(*bsa), UINT32_MAX)) {
+			goto skip_checks;
+		}
+
 		ps = bsa->ps;
 		pc = (void *)bsa->pc;
 
 		/* Default for exception */
-		int reason = K_ERR_CPU_EXCEPTION;
 		is_fatal_error = true;
 
 		/* We need to distinguish between an ill in xtensa_arch_except,
@@ -297,6 +429,7 @@ void *xtensa_excint1_c(void *esf)
 			}
 		}
 
+skip_checks:
 		if (reason != K_ERR_KERNEL_OOPS) {
 			print_fatal_exception(print_stack, cause, is_dblexc, depc);
 		}

arch/xtensa/include/xtensa_stack.h

@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2024 Intel Corporation
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef ZEPHYR_ARCH_XTENSA_XTENSA_STACK_H_
+#define ZEPHYR_ARCH_XTENSA_XTENSA_STACK_H_
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#include <xtensa_asm2_context.h>
+
+/**
+ * @defgroup xtensa_stack_internal_apis Xtensa Stack related Internal APIs
+ * @ingroup xtensa_stack_apis
+ * @{
+ */
+
+/**
+ * @brief Check if memory region is within correct stack boundaries.
+ *
+ * Check if the memory region [@a addr, (@a addr + @a sz)) is within
+ * correct stack boundaries:
+ * - Interrupt stack if servicing interrupts.
+ * - Privileged stack if in kernel mode doing syscalls.
+ * - Thread stack otherwise.
+ *
+ * @note When @ps == UINT32_MAX, it checks the whole range of stack
+ *       object because we cannot get PS via frame pointer yet.
+ *
+ * @param addr Beginning address of memory region to check.
+ * @param sz Size of memory region to check. Can be zero.
+ * @param ps PS register value of interrupted context. Use UINT32_MAX if
+ *           PS cannot be determined at time of call.
+ *
+ * @return True if memory region is outside stack bounds, false otherwise.
+ */
+bool xtensa_is_outside_stack_bounds(uintptr_t addr, size_t sz, uint32_t ps);
+
+/**
+ * @brief Check if frame pointer is within correct stack boundaries.
+ *
+ * Check if the frame pointer and its associated BSA (base save area) are
+ * within correct stack boundaries. Use @ref xtensa_is_outside_stack_bounds
+ * to determine validity.
+ *
+ * @param frame Frame Pointer. Cannot be NULL.
+ */
+bool xtensa_is_frame_pointer_valid(_xtensa_irq_stack_frame_raw_t *frame);
+
+/**
+ * @}
+ */
+
+#endif /* ZEPHYR_ARCH_XTENSA_XTENSA_STACK_H_ */