[PATCH v3] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()

* [PATCH v3] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
@ 2026-06-25 12:39 ` Jiakai Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Jiakai Xu @ 2026-06-25 12:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Jiakai Xu,
	Matthew Bystrin, Palmer Dabbelt, Paul Walmsley, Rui Qi,
	Samuel Holland, Jiakai Xu

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 the absolute task stack boundary (task_pt_regs(task))
when sp is on the task stack. When CONFIG_IRQ_STACKS is enabled and
sp is on the IRQ stack, use the IRQ stack's top as the upper bound
instead, since task_pt_regs() points to the task stack's pt_regs
which is on a different stack and would give a wrong boundary.

This check is done once before the unwind loop, based on the initial
sp value. When unwinding starts on the IRQ stack, the IRQ stack
boundary is used for all frames, including those on the task stack
reached after crossing the call_on_irq_stack() transition. This is
safe because the IRQ stack and task stack are separate allocations;
the IRQ stack boundary will not incorrectly reject task stack frames
that are below it.

Fixes: a2a4d4a6a0bf ("riscv: stacktrace: fixed walk_stackframe()")
Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
Assisted-by: YuanSheng:DeepSeek-V3.2
---
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.
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.
https://lore.kernel.org/linux-riscv/20260514100711.838895-1-xujiakai2025@iscas.ac.cn/t/#u
---
 arch/riscv/kernel/stacktrace.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index c7555447149b..b47a8acf4759 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -35,12 +35,16 @@
 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)
+#ifdef CONFIG_IRQ_STACKS
+DECLARE_PER_CPU(ulong *, irq_stack_ptr);
+#endif
+
+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 +52,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;
 	int graph_idx = 0;
 	int level = 0;
 
@@ -68,19 +72,32 @@ 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 (IS_ENABLED(CONFIG_IRQ_STACKS)) {
+		high = (unsigned long)this_cpu_read(irq_stack_ptr) +
+		       IRQ_STACK_SIZE;
+	} else {
+		high = (unsigned long)task_pt_regs(task);
+	}
+
 	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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 10+ messages in thread