[PATCH v5] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
Jiakai Xu
xujiakai2025 at iscas.ac.cn
Thu Aug 27 05:38:48 PDT 2026
gentle ping ?
> The fp_is_valid() function uses ALIGN(sp, THREAD_SIZE) as the upper
> bound for the frame pointer check. This bound is calculated relative
> to the current sp and shifts upward when sp itself exceeds the valid
> stack region, allowing the unwinder to read past the end of the
> allocated task stack and triggering KASAN stack-out-of-bounds.
>
> Fix this by using absolute stack boundaries determined once before
> the unwind loop:
>
> - When sp is on the task stack, use the task's pt_regs as the upper
> bound.
> - When sp is on the overflow_stack (CONFIG_VMAP_STACK=y), use the
> overflow_stack's top as the boundary.
> - When sp is on the IRQ stack (CONFIG_IRQ_STACKS=y), use the IRQ
> stack's top as the boundary.
> - When sp is not on any known stack, warn and return.
> - For remote tasks (task != current), if sp is not on the task
> stack, warn and return since we cannot reliably determine the
> correct boundary from a different CPU's stacks.
>
> Make the DECLARE_PER_CPU(overflow_stack) unconditional in
> asm/stacktrace.h so that stacktrace.c can use
> IS_ENABLED(CONFIG_VMAP_STACK) instead of #ifdef, in line with the
> kernel coding style which discourages the use of #ifdef in .c files
> (https://docs.kernel.org/process/coding-style.html). This is safe
> because the DEFINE_PER_CPU (memory allocation) in traps.c remains
> guarded by CONFIG_VMAP_STACK; the reference in stacktrace.c is only
> compiled when IS_ENABLED(CONFIG_VMAP_STACK) evaluates to true.
>
> Fixes: a2a4d4a6a0bf ("riscv: stacktrace: fixed walk_stackframe()")
> Signed-off-by: Jiakai Xu <xujiakai2025 at iscas.ac.cn>
> Assisted-by: YuanSheng:DeepSeek-V3.2
> ---
> V4 -> V5:
> - Made DECLARE_PER_CPU(overflow_stack) unconditional in
> asm/stacktrace.h so that stacktrace.c can use IS_ENABLED()
> instead of #ifdef, as suggested by Nam Cao.
> - Converted #ifdef CONFIG_VMAP_STACK to else if
> (IS_ENABLED(CONFIG_VMAP_STACK)) in walk_stackframe() and folded
> the overflow_stack and IRQ stack checks into a single if-else
> chain, eliminating the !high sentinel variable.
> Link: https://lore.kernel.org/linux-riscv/20260630090535.4103888-1-xujiakai2025@iscas.ac.cn/T/#u
>
> V3 -> V4:
> - Switched to #include <asm/irq_stack.h> instead of hand-written
> DECLARE_PER_CPU, as suggested by Nam Cao. This also fixes a
> build failure when CONFIG_IRQ_STACKS=n since the header
> unconditionally declares irq_stack_ptr.
> - Added dedicated overflow_stack handling (CONFIG_VMAP_STACK)
> with the correct 4 KiB boundary. v3 fell through to the 32 KiB
> IRQ stack bound when unwinding from the overflow_stack,
> triggering KASAN OOB reads past the small overflow_stack
> allocation. Reported by Sashiko AI review and reproduced by Xiao Wu.
> - Changed the fallback else branch to warn and return instead of
> silently falling back to task_pt_regs(), as suggested by Nam Cao.
> - For remote tasks (task != current), bail out with a warning when
> sp is not on the task stack, since we cannot reliably use the
> local CPU's IRQ/overflow stacks as the boundary.
> Link: https://lore.kernel.org/linux-riscv/20260625123906.211981-1-xujiakai2025@iscas.ac.cn/T/#u
>
> V2 -> V3:
> - Handled the case where sp is on the IRQ stack (CONFIG_IRQ_STACKS),
> as suggested by Nam Cao: when sp is not within the task stack
> range, use the IRQ stack's top (irq_stack_ptr + IRQ_STACK_SIZE)
> as the upper bound instead of task_pt_regs(), which would point
> to the wrong stack. The check uses the sp variable directly rather
> than on_thread_stack(), because on_thread_stack() reads the
> current CPU register sp which may differ from the unwinder's sp
> when regs is provided.
> - Used IS_ENABLED(CONFIG_IRQ_STACKS) to guard the IRQ stack check,
> and cast irq_stack_ptr to unsigned long for byte-level arithmetic.
> Link: https://lore.kernel.org/linux-riscv/20260517143704.659416-1-xujiakai2025@iscas.ac.cn/T/#u
>
> V1 -> V2:
> - Moved the NULL task check from fp_is_valid() into walk_stackframe(),
> as suggested by Matthew Bystrin.
> - Changed the upper bound from task_stack_page(task) + THREAD_SIZE to
> task_pt_regs(task) for a tighter boundary, as suggested by Matthew
> Bystrin.
> Link: https://lore.kernel.org/linux-riscv/20260514100711.838895-1-xujiakai2025@iscas.ac.cn/t/#u
> ---
> arch/riscv/include/asm/stacktrace.h | 8 +++++--
> arch/riscv/kernel/stacktrace.c | 37 ++++++++++++++++++++++++-----
> 2 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h
> index b1495a7e06ce..2d13c35a8c05 100644
> --- a/arch/riscv/include/asm/stacktrace.h
> +++ b/arch/riscv/include/asm/stacktrace.h
> @@ -22,8 +22,12 @@ static inline bool on_thread_stack(void)
> }
>
>
> -#ifdef CONFIG_VMAP_STACK
> +/*
> + * Declare unconditionally so that arch/riscv/kernel/stacktrace.c can use
> + * IS_ENABLED(CONFIG_VMAP_STACK) rather than #ifdef, in line with the kernel
> + * coding style. The DEFINE_PER_CPU (memory allocation) remains guarded by
> + * CONFIG_VMAP_STACK in traps.c.
> + */
> DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
> -#endif /* CONFIG_VMAP_STACK */
>
> #endif /* _ASM_RISCV_STACKTRACE_H */
> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index c7555447149b..7a66c7ee2c81 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -12,6 +12,7 @@
> #include <linux/stacktrace.h>
> #include <linux/ftrace.h>
>
> +#include <asm/irq_stack.h>
> #include <asm/stacktrace.h>
>
> #ifdef CONFIG_FRAME_POINTER
> @@ -35,12 +36,12 @@
> extern asmlinkage void handle_exception(void);
> extern unsigned long ret_from_exception_end;
>
> -static inline int fp_is_valid(unsigned long fp, unsigned long sp)
> +static inline int fp_is_valid(unsigned long fp, unsigned long sp,
> + unsigned long high)
> {
> - unsigned long low, high;
> + unsigned long low;
>
> low = sp + sizeof(struct stackframe);
> - high = ALIGN(sp, THREAD_SIZE);
>
> return !(fp < low || fp > high || fp & 0x07);
> }
> @@ -48,7 +49,7 @@ static inline int fp_is_valid(unsigned long fp, unsigned long sp)
> void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
> bool (*fn)(void *, unsigned long), void *arg)
> {
> - unsigned long fp, sp, pc;
> + unsigned long fp, sp, pc, high = 0;
> int graph_idx = 0;
> int level = 0;
>
> @@ -68,19 +69,43 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
> pc = task->thread.ra;
> }
>
> + if (!task)
> + task = current;
> +
> + if (sp >= (unsigned long)task_stack_page(task) &&
> + sp < (unsigned long)task_stack_page(task) + THREAD_SIZE) {
> + high = (unsigned long)task_pt_regs(task);
> + } else if (task != current) {
> + pr_warn("%s: sp (%lx) is not in task stack of %s\n",
> + __func__, sp, task->comm);
> + return;
> + } else if (IS_ENABLED(CONFIG_VMAP_STACK) &&
> + sp >= (unsigned long)this_cpu_ptr(overflow_stack) &&
> + sp < (unsigned long)this_cpu_ptr(overflow_stack) + OVERFLOW_STACK_SIZE) {
> + high = (unsigned long)this_cpu_ptr(overflow_stack) + OVERFLOW_STACK_SIZE;
> + } else if (IS_ENABLED(CONFIG_IRQ_STACKS) &&
> + sp >= (unsigned long)this_cpu_read(irq_stack_ptr) &&
> + sp < (unsigned long)this_cpu_read(irq_stack_ptr) + IRQ_STACK_SIZE) {
> + high = (unsigned long)this_cpu_read(irq_stack_ptr) + IRQ_STACK_SIZE;
> + } else {
> + pr_warn("%s: sp (%lx) is not on any known stack\n",
> + __func__, sp);
> + return;
> + }
> +
> for (;;) {
> struct stackframe *frame;
>
> if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc))))
> break;
>
> - if (unlikely(!fp_is_valid(fp, sp)))
> + if (unlikely(!fp_is_valid(fp, sp, high)))
> break;
>
> /* Unwind stack frame */
> frame = (struct stackframe *)fp - 1;
> sp = fp;
> - if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp)) {
> + if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp, high)) {
> /* We hit function where ra is not saved on the stack */
> fp = frame->ra;
> pc = regs->ra;
> --
> 2.34.1
More information about the linux-riscv
mailing list