[PATCH v7 03/10] riscv: ftrace: use the frame pointer as the function graph return-address cookie

[Shuai Xue]

The function graph tracer stamps every shadow-stack entry with a
cookie, ftrace_ret_stack::retp, which stack unwinders later pass to
ftrace_graph_ret_addr() to recover a return address that the tracer
replaced with return_to_handler.  The match is an exact pointer
comparison, so the producer and the consumer must agree on what the
cookie is.  On riscv they don't:

  * The dynamic ftrace path stores &arch_ftrace_regs(fregs)->ra, a
    slot inside the transient ftrace trampoline frame.  The frame is
    popped as soon as the callback returns, so the stored cookie is
    an address that no consumer can ever reproduce.  It has never
    matched anything since ftrace_graph_func() was introduced in
    v6.8.

  * The frame-pointer walker in walk_stackframe() passes &frame->ra,
    the ra slot of a live frame record.  That key matches the static
    mcount path, which passes the callee's ra-slot address, but not
    the dynamic path.

Since distro kernels only ever use the dynamic path, the net effect
is that the shadow stack is never consulted during unwinding:
whenever the function graph tracer is running, dump_stack(), perf
callchains and __get_wchan() report return_to_handler instead of the
real caller.

Switch the cookie to the architectural frame pointer, i.e. the
caller's frame-pointer value.  Every party can reproduce it from the
frame records themselves:

  * ftrace.c stores (void *)frame_pointer on both entry paths.  At
    ftrace entry the frame pointer is still the caller's, which is
    the value the callee later saves in its frame_record::fp.

  * walk_stackframe() passes that value, frame->fp, to
    ftrace_graph_ret_addr() instead of the ra-slot address.  The
    static mcount path has to switch too: its old cookie (the
    callee's ra-slot address) is exactly what the old walker key
    matched, so leaving it behind would regress the static path.

This is the convention arm64 already uses: (void *)frame_pointer
from ftrace.c, the saved fp from the unwinder.

function_get_true_parent_ip() keeps passing the entry stack pointer
as its key; that key is only reproducible on architectures whose
return address lives at the entry stack pointer (x86).  On riscv,
as on arm64, it keeps falling back to the raw parent ip.

Fixes: 35e61e8827ee8 ("riscv: ftrace: Make function graph use ftrace directly")
Suggested-by: Martin Kaiser <martin at kaiser.cx>
Cc: stable at vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai at linux.alibaba.com>
---
 arch/riscv/kernel/ftrace.c     | 6 ++++--
 arch/riscv/kernel/stacktrace.c | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index be8b68514417..0facd824c911 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -245,7 +245,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 	 */
 	old = *parent;
 
-	if (!function_graph_enter(old, self_addr, frame_pointer, parent))
+	if (!function_graph_enter(old, self_addr, frame_pointer,
+				  (void *)frame_pointer))
 		*parent = return_hooker;
 }
 
@@ -267,7 +268,8 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 	 */
 	old = *parent;
 
-	if (!function_graph_enter_regs(old, ip, frame_pointer, parent, fregs))
+	if (!function_graph_enter_regs(old, ip, frame_pointer,
+				       (void *)frame_pointer, fregs))
 		*parent = return_hooker;
 }
 #endif /* CONFIG_DYNAMIC_FTRACE */
diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 28c5744889ba..a5abb705d43b 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -96,8 +96,9 @@ static __always_inline void __walk_stackframe(struct task_struct *task,
 		} else {
 			fp = READ_ONCE_TASK_STACK(task, frame->fp);
 			pc = READ_ONCE_TASK_STACK(task, frame->ra);
+			/* the cookie is the caller's frame pointer */
 			pc = ftrace_graph_ret_addr(task, &graph_idx, pc,
-						   &frame->ra);
+						   (void *)fp);
 			if (pc >= (unsigned long)handle_exception &&
 			    pc < (unsigned long)&ret_from_exception_end) {
 				if (unlikely(!fn(arg, pc)))
-- 
2.39.3