All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
@ 2026-05-17 14:37 ` Jiakai Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Jiakai Xu @ 2026-05-17 14:37 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Jiakai Xu,
	Matthew Bystrin, Palmer Dabbelt, Paul Walmsley, Samuel Holland

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)) instead. This ensures that once the frame pointer 
walks past the actual end of the stack, the check consistently fails 
and the unwinding terminates.

Fixes: a2a4d4a6a0bf ("riscv: stacktrace: fixed walk_stackframe()")
Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
Assisted-by: OpenClaw:DeepSeek-V3.2
---
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.
---
 arch/riscv/kernel/stacktrace.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index b41b6255751c..6ce3465bec2c 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -35,12 +35,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 +48,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 +68,24 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		pc = task->thread.ra;
 	}
 
+	if (!task)
+		task = current;
+
+	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] 8+ messages in thread
* [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
@ 2026-05-17 14:37 ` Jiakai Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Jiakai Xu @ 2026-05-17 14:37 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Jiakai Xu,
	Matthew Bystrin, Palmer Dabbelt, Paul Walmsley, Samuel Holland

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)) instead. This ensures that once the frame pointer 
walks past the actual end of the stack, the check consistently fails 
and the unwinding terminates.

Fixes: a2a4d4a6a0bf ("riscv: stacktrace: fixed walk_stackframe()")
Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
Assisted-by: OpenClaw:DeepSeek-V3.2
---
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.
---
 arch/riscv/kernel/stacktrace.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index b41b6255751c..6ce3465bec2c 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -35,12 +35,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 +48,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 +68,24 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		pc = task->thread.ra;
 	}
 
+	if (!task)
+		task = current;
+
+	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


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
  2026-05-17 14:37 ` Jiakai Xu
@ 2026-05-18 20:37   ` Matthew Bystrin
  -1 siblings, 0 replies; 8+ messages in thread
From: Matthew Bystrin @ 2026-05-18 20:37 UTC (permalink / raw)
  To: Jiakai Xu, linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Matthew Bystrin,
	Palmer Dabbelt, Paul Walmsley, Samuel Holland

Hi!

> @@ -68,19 +68,24 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>  		pc = task->thread.ra;
>  	}
>  
> +	if (!task)
> +		task = current;
> +
> +	high = (unsigned long)task_pt_regs(task);

After noticing end_of_stack() function I'm not sure task_pt_regs() is the best
candidate for the boundary. Let's see what maintainers are going to say.

> +
>  	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

All in all looks fine.

Reviewed-by: Matthew Bystrin <dev.mbstr@gmail.com>

-- 
Best regards
Matt

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
@ 2026-05-18 20:37   ` Matthew Bystrin
  0 siblings, 0 replies; 8+ messages in thread
From: Matthew Bystrin @ 2026-05-18 20:37 UTC (permalink / raw)
  To: Jiakai Xu, linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Matthew Bystrin,
	Palmer Dabbelt, Paul Walmsley, Samuel Holland

Hi!

> @@ -68,19 +68,24 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>  		pc = task->thread.ra;
>  	}
>  
> +	if (!task)
> +		task = current;
> +
> +	high = (unsigned long)task_pt_regs(task);

After noticing end_of_stack() function I'm not sure task_pt_regs() is the best
candidate for the boundary. Let's see what maintainers are going to say.

> +
>  	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

All in all looks fine.

Reviewed-by: Matthew Bystrin <dev.mbstr@gmail.com>

-- 
Best regards
Matt

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
  2026-05-17 14:37 ` Jiakai Xu
@ 2026-06-12  1:41   ` Jiakai Xu
  -1 siblings, 0 replies; 8+ messages in thread
From: Jiakai Xu @ 2026-06-12  1:41 UTC (permalink / raw)
  To: xujiakai2025
  Cc: alex, aou, dev.mbstr, linux-kernel, linux-riscv, palmer, pjw,
	samuel.holland, zhangchunyan

Hi all,

Gentle ping on this patch.

Thanks,
Jiakai


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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
@ 2026-06-12  1:41   ` Jiakai Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Jiakai Xu @ 2026-06-12  1:41 UTC (permalink / raw)
  To: xujiakai2025
  Cc: alex, aou, dev.mbstr, linux-kernel, linux-riscv, palmer, pjw,
	samuel.holland, zhangchunyan

Hi all,

Gentle ping on this patch.

Thanks,
Jiakai


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
  2026-05-18 20:37   ` Matthew Bystrin
  (?)
@ 2026-06-19  9:11   ` Nam Cao
  -1 siblings, 0 replies; 8+ messages in thread
From: Nam Cao @ 2026-06-19  9:11 UTC (permalink / raw)
  To: Matthew Bystrin, Jiakai Xu, linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Matthew Bystrin,
	Palmer Dabbelt, Paul Walmsley, Samuel Holland

"Matthew Bystrin" <dev.mbstr@gmail.com> writes:
>> +	high = (unsigned long)task_pt_regs(task);
>
> After noticing end_of_stack() function I'm not sure task_pt_regs() is the best
> candidate for the boundary. Let's see what maintainers are going to say.

end_of_stack() is the lower bound of the stack, that's not what we want.

Nam

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
  2026-05-17 14:37 ` Jiakai Xu
                   ` (2 preceding siblings ...)
  (?)
@ 2026-06-19  9:51 ` Nam Cao
  -1 siblings, 0 replies; 8+ messages in thread
From: Nam Cao @ 2026-06-19  9:51 UTC (permalink / raw)
  To: Jiakai Xu, linux-kernel, linux-riscv
  Cc: Albert Ou, Alexandre Ghiti, Chunyan Zhang, Jiakai Xu,
	Matthew Bystrin, Palmer Dabbelt, Paul Walmsley, Samuel Holland

Jiakai Xu <xujiakai2025@iscas.ac.cn> writes:
> +	if (!task)
> +		task = current;
> +
> +	high = (unsigned long)task_pt_regs(task);

I am concerned when CONFIG_IRQ_STACKS=y and we are on the irq stack,
wouldn't this "high" be the wrong one?

I haven't validated it, we probably need something like

if (on_thread_stack()) {
	if (!task)
        	task = current;
	high = (unsigned long)task_pt_regs(task);
} else {
	high = per_cpu(irq_stack_ptr, smp_processor_id()) + IRQ_STACK_SIZE/sizeof(ulong);
}

Nam

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-19  9:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 14:37 [PATCH v2] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe() Jiakai Xu
2026-05-17 14:37 ` Jiakai Xu
2026-05-18 20:37 ` Matthew Bystrin
2026-05-18 20:37   ` Matthew Bystrin
2026-06-19  9:11   ` Nam Cao
2026-06-12  1:41 ` Jiakai Xu
2026-06-12  1:41   ` Jiakai Xu
2026-06-19  9:51 ` Nam Cao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.