Re: [PATCH bpf-next 1/3] riscv: stacktrace: Implement arch_bpf_stack_walk() for BPF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `ftrace_graph_ret_addr()` resolution in `arch_bpf_stack_walk()` breaks BPF exception unwinding when the ftrace function graph tracer is enabled.
--

commit 18f97d2c8c3fdcb52341a066f5cdbb33e67828f4
Author: Varun R Mallya <varunrmallya@gmail.com>

riscv: stacktrace: Implement arch_bpf_stack_walk() for BPF
    
This commit implements arch_bpf_stack_walk() for RISC-V when
CONFIG_FRAME_POINTER is enabled. It allows unwinding the stack up to the
program marked as the exception boundary, which is required to support BPF
exceptions on this architecture.

> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index b41b6255751cb..a4c2411f4038b 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c

[ ... ]

> @@ -102,6 +103,33 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>  	}
>  }
>  
> +void notrace arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp),
> +				 void *cookie)
> +{
> +	unsigned long fp, sp, pc;
> +
> +	fp = (unsigned long)__builtin_frame_address(0);
> +	sp = current_stack_pointer;
> +	pc = (unsigned long)arch_bpf_stack_walk;
> +
> +	for (;;) {
> +		struct stackframe *frame;
> +
> +		if (unlikely(!__kernel_text_address(pc)))
> +			break;
> +		/* pc belongs to the function whose frame pointer is fp */
> +		if (!consume_fn(cookie, pc, sp, fp))
> +			break;
> +		if (unlikely(!fp_is_valid(fp, sp)))
> +			break;
> +
> +		frame = (struct stackframe *)fp - 1;
> +		sp = fp;
> +		fp = READ_ONCE_TASK_STACK(current, frame->fp);
> +		pc = READ_ONCE_TASK_STACK(current, frame->ra);

[Severity: High]
Does this code need to resolve the ftrace trampoline address?

Looking at the generic unwinder walk_stackframe() in the same file, it uses
ftrace_graph_ret_addr() to recover the real instruction pointer when the
ftrace function graph tracer is active.

If a kernel function in the unwinding chain is traced by ftrace, frame->ra
will point to return_to_handler on the stack. Because return_to_handler is a
core kernel text address rather than a BPF program address, the stack walker
might silently skip the caller's frame and miss the BPF exception boundary.
This would leave the context uninitialized and trigger a kernel panic in
bpf_throw() when it validates the missing boundary.

Could pc be passed through ftrace_graph_ret_addr() here to ensure the real
return address is consumed?

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260621144259.288135-1-varunrmallya@gmail.com?part=1