* [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch
@ 2026-09-14 9:26 Shuai Xue
2026-09-14 9:26 ` [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata Shuai Xue
` (8 more replies)
0 siblings, 9 replies; 39+ messages in thread
From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw)
To: palmer, pjw, aou
Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz,
mingo, acme, namhyung, alexander.shishkin, jolsa, irogers,
adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek,
joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai,
jkchen, linux-kernel, linux-trace-kernel, linux-perf-users,
live-patching, linux-kselftest
Hi,
This is v6 of the RISC-V reliable stack unwinding series for livepatch.
Wang Han has left our team, so I am taking over this series from now on.
The series adds explicit frame-record metadata, preserves the frame
pointer through dynamic ftrace, and replaces the open-coded stack walker
with a conservative frame-pointer based unwinder. It then enables the
RISC-V livepatch capability and adds the syscall-wrapper selftest prefix.
Changes since v5:
* Rebase onto the current riscv/for-next; no functional changes.
* Fold in the tags collected from the v5 thread:
- Reviewed-by: Shuai Xue on 2/7, 3/7, 4/7 and 6/7
- Acked-by: Miroslav Benes on 7/7
* Add my sign-off as the new sender.
Since the patches are otherwise identical to v5, the v5 validation
results still apply. For reference, from the v5 cover letter:
* Built Linux 7.2.0-rc6 with riscv64-unknown-linux-gnu-gcc 15.2.0
and binutils 2.46.
* Standard livepatch configuration built successfully, including
Image, vmlinux, modules, compile_commands.json and GDB scripts.
* CONFIG_IRQ_STACKS=n variant linked successfully with GCC.
* QEMU 8.2.2, 1 CPU: livepatch selftests PASS: 7, SKIP: 1,
FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported.
* QEMU 8.2.2, 4 CPUs: livepatch selftests PASS: 7, SKIP: 1,
FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported.
* No BUG, WARNING, Oops, panic, unreliable stack or Call Trace found
by the validation log scans. Expected negative livepatch errors are
part of the selftests.
v5 was posted in early August and has been quiet since then. I would
appreciate it if the series could be considered for the upcoming merge
window. Happy to address any remaining feedback.
Base:
riscv/for-next 77ae27fd98f3 ("Merge tag 'printk-for-7.3' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux")
Previous versions:
v5: https://lore.kernel.org/r/20260804103626.2803112-1-wanghan@linux.alibaba.com
v4 RESEND: https://lore.kernel.org/r/20260629072713.3273743-1-wanghan@linux.alibaba.com
v3: https://lore.kernel.org/r/cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com
v2: https://lore.kernel.org/r/20260528082310.1994388-1-wanghan@linux.alibaba.com
v1: https://lore.kernel.org/r/20260527123530.2593918-1-wanghan@linux.alibaba.com
Assisted-by: Qoder:Qwen-3.8-MAX-Preview
Wang Han (7):
riscv: stacktrace: Add frame record metadata
riscv: stacktrace: disable KASAN and KCOV instrumentation for
stacktrace.o
riscv: ftrace: always preserve s0 in dynamic ftrace register frame
riscv: stacktrace: introduce stack-bound tracking helpers
riscv: stacktrace: switch to frame-pointer based unwinder
riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
selftests/livepatch: Add RISC-V syscall wrapper prefix
arch/riscv/Kconfig | 4 +
arch/riscv/include/asm/ptrace.h | 9 +
arch/riscv/include/asm/stacktrace.h | 67 ++-
arch/riscv/include/asm/stacktrace/common.h | 159 +++++
arch/riscv/include/asm/stacktrace/frame.h | 53 ++
arch/riscv/kernel/Makefile | 6 +
arch/riscv/kernel/asm-offsets.c | 6 +
arch/riscv/kernel/entry.S | 39 +-
arch/riscv/kernel/ftrace.c | 6 +-
arch/riscv/kernel/head.S | 23 +
arch/riscv/kernel/mcount-dyn.S | 4 -
arch/riscv/kernel/perf_callchain.c | 2 +-
arch/riscv/kernel/process.c | 33 +-
arch/riscv/kernel/stacktrace.c | 561 +++++++++++++++---
.../livepatch/test_modules/test_klp_syscall.c | 2 +
15 files changed, 868 insertions(+), 106 deletions(-)
create mode 100644 arch/riscv/include/asm/stacktrace/common.h
create mode 100644 arch/riscv/include/asm/stacktrace/frame.h
base-commit: 77ae27fd98f3b9ab376239051ec7f9ee34c2c072
--
2.39.3
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:40 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue ` (7 subsequent siblings) 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Reliable frame-pointer unwinding needs an explicit way to identify exception boundaries and the final entry frame. The existing unwinder infers those boundaries from return addresses, which is too loose for a future reliable unwinder. Add a small metadata frame record to pt_regs and initialize it on exception entry, kernel stack overflow, kernel thread fork, user fork, and early idle task setup. The record uses a zero {fp, ra} sentinel plus a type field so a later unwinder can distinguish a final user-to-kernel boundary from a nested kernel pt_regs boundary. This follows the arm64 metadata frame-record model, adapted to the RISC-V {fp, ra} frame record convention. The metadata is established at the RISC-V entry boundaries that need an explicit unwind marker: * exception entry clears the metadata {fp, ra} pair and uses SPP (or MPP in M-mode) to record whether the pt_regs frame is the final user-to-kernel boundary or a nested kernel boundary; * the kernel stack overflow path builds a nested pt_regs metadata record on the overflow stack so an unwinder can resume from the pre-overflow s0 saved in PT_S0; * _start_kernel builds the init task's final metadata record, while the secondary CPU path sets up s0 before smp_callin() so idle-task unwinding does not inherit an undefined caller frame; * copy_thread creates matching final metadata records for new kernel and user tasks, and keeps s0 available for the frame-pointer chain. Keep the embedded metadata-record field offsets distinct from the s0-relative STACKFRAME_* offsets used by call_on_irq_stack(), because the latter describe a frame record relative to s0 rather than to the record base. These changes keep s0 reserved for the frame-pointer chain at task and exception boundaries. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/include/asm/ptrace.h | 9 ++++ arch/riscv/include/asm/stacktrace/frame.h | 53 +++++++++++++++++++++++ arch/riscv/kernel/asm-offsets.c | 6 +++ arch/riscv/kernel/entry.S | 39 ++++++++++++++++- arch/riscv/kernel/head.S | 23 ++++++++++ arch/riscv/kernel/process.c | 33 +++++++++++++- 6 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/frame.h diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h index addc8188152f7..4b9b0f2792143 100644 --- a/arch/riscv/include/asm/ptrace.h +++ b/arch/riscv/include/asm/ptrace.h @@ -8,6 +8,7 @@ #include <uapi/asm/ptrace.h> #include <asm/csr.h> +#include <asm/stacktrace/frame.h> #include <linux/compiler.h> #ifndef __ASSEMBLER__ @@ -53,6 +54,14 @@ struct pt_regs { unsigned long cause; /* a0 value before the syscall */ unsigned long orig_a0; + + /* + * This frame record is entirely zeroed on exception entry, allowing the + * unwinder to identify exception boundaries. The type field encodes + * whether the exception was taken from user (FINAL) or kernel (PT_REGS) + * mode. + */ + struct frame_record_meta stackframe; }; #define PTRACE_SYSEMU 0x1f diff --git a/arch/riscv/include/asm/stacktrace/frame.h b/arch/riscv/include/asm/stacktrace/frame.h new file mode 100644 index 0000000000000..5720a6c65fe88 --- /dev/null +++ b/arch/riscv/include/asm/stacktrace/frame.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_RISCV_STACKTRACE_FRAME_H +#define __ASM_RISCV_STACKTRACE_FRAME_H + +/* + * See: arch/arm64/include/asm/stacktrace/frame.h for the reference + * implementation. + */ + +/* + * - FRAME_META_TYPE_NONE + * + * This value is reserved. + * + * - FRAME_META_TYPE_FINAL + * + * The record is the last entry on the stack. + * Unwinding should terminate successfully. + * + * - FRAME_META_TYPE_PT_REGS + * + * The record is embedded within a struct pt_regs, recording the registers at + * an arbitrary point in time. + * Unwinding should consume pt_regs::epc, followed by pt_regs::ra. + * + * Note: all other values are reserved and should result in unwinding + * terminating with an error. + */ +#define FRAME_META_TYPE_NONE 0 +#define FRAME_META_TYPE_FINAL 1 +#define FRAME_META_TYPE_PT_REGS 2 + +#ifndef __ASSEMBLER__ +/* + * A standard RISC-V frame record. + */ +struct frame_record { + unsigned long fp; + unsigned long ra; +}; + +/* + * A metadata frame record indicating a special unwind. + * The record::{fp,ra} fields must be zero to indicate the presence of + * metadata. + */ +struct frame_record_meta { + struct frame_record record; + unsigned long type; +}; +#endif /* __ASSEMBLER__ */ + +#endif /* __ASM_RISCV_STACKTRACE_FRAME_H */ diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c index a75f0cfea1e9f..d0eaf2c902fb2 100644 --- a/arch/riscv/kernel/asm-offsets.c +++ b/arch/riscv/kernel/asm-offsets.c @@ -131,6 +131,9 @@ void asm_offsets(void) OFFSET(PT_BADADDR, pt_regs, badaddr); OFFSET(PT_CAUSE, pt_regs, cause); + DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe)); + DEFINE(S_STACKFRAME_TYPE, offsetof(struct pt_regs, stackframe.type)); + OFFSET(SUSPEND_CONTEXT_REGS, suspend_context, regs); OFFSET(HIBERN_PBE_ADDR, pbe, address); @@ -503,6 +506,9 @@ void asm_offsets(void) DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN)); DEFINE(STACKFRAME_FP, offsetof(struct stackframe, fp) - sizeof(struct stackframe)); DEFINE(STACKFRAME_RA, offsetof(struct stackframe, ra) - sizeof(struct stackframe)); + DEFINE(STACKFRAME_RECORD_SIZE, sizeof(struct frame_record)); + OFFSET(FRAME_RECORD_FP, frame_record, fp); + OFFSET(FRAME_RECORD_RA, frame_record, ra); #ifdef CONFIG_FUNCTION_TRACER DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S index d799c4e56f804..588d5bbe74f56 100644 --- a/arch/riscv/kernel/entry.S +++ b/arch/riscv/kernel/entry.S @@ -11,6 +11,7 @@ #include <asm/asm.h> #include <asm/csr.h> #include <asm/scs.h> +#include <asm/stacktrace/frame.h> #include <asm/unistd.h> #include <asm/page.h> #include <asm/thread_info.h> @@ -198,6 +199,27 @@ SYM_CODE_START(handle_exception) REG_S s4, PT_CAUSE(sp) REG_S s5, PT_TP(sp) + /* + * Create a metadata frame record. The unwinder will use this to + * identify and unwind exception boundaries. + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) /* stackframe.record.fp = 0 */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) /* stackframe.record.ra = 0 */ +#ifdef CONFIG_RISCV_M_MODE + li t0, SR_MPP + and t0, s1, t0 +#else + andi t0, s1, SR_SPP +#endif + bnez t0, 1f + li t0, FRAME_META_TYPE_FINAL + j 2f +1: + li t0, FRAME_META_TYPE_PT_REGS +2: + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + /* * Set the scratch register to 0, so that if a recursive exception * occurs, the exception vector knows it came from the kernel @@ -354,6 +376,19 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow) REG_S s3, PT_BADADDR(sp) REG_S s4, PT_CAUSE(sp) REG_S s5, PT_TP(sp) + + /* + * Create a metadata frame record for the overflow pt_regs. The + * overflow path is entered from kernel context, so this is a nested + * pt_regs boundary and the unwinder can resume from the pre-overflow + * frame pointer saved in PT_S0. + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) + li t0, FRAME_META_TYPE_PT_REGS + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + move a0, sp tail handle_bad_stack SYM_CODE_END(handle_kernel_stack_overflow) @@ -362,8 +397,8 @@ ASM_NOKPROBE(handle_kernel_stack_overflow) SYM_CODE_START(ret_from_fork_kernel_asm) call schedule_tail - move a0, s1 /* fn_arg */ - move a1, s0 /* fn */ + move a0, s3 /* fn_arg */ + move a1, s2 /* fn */ move a2, sp /* pt_regs */ call ret_from_fork_kernel j ret_from_exception diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index f6a8ca49e6277..341b2d3facbc0 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -14,6 +14,7 @@ #include <asm/hwcap.h> #include <asm/image.h> #include <asm/scs.h> +#include <asm/stacktrace/frame.h> #include <asm/usercfi.h> #include "efi-header.S" @@ -177,6 +178,14 @@ secondary_start_sbi: REG_S a0, (a1) 1: #endif + + /* + * Set up the frame pointer for the secondary idle task so reliable + * stack unwinding terminates at the metadata frame in task_pt_regs(). + * Without this, the first frame records can inherit an undefined caller + * fp and unwind past smp_callin() into .Lsecondary_park. + */ + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE scs_load_current call smp_callin #endif /* CONFIG_SMP */ @@ -305,6 +314,20 @@ SYM_CODE_START(_start_kernel) la tp, init_task la sp, init_thread_union + THREAD_SIZE addi sp, sp, -PT_SIZE_ON_STACK + + /* + * Set up a metadata frame record for the init task so that + * the unwinder can identify the outermost frame by its + * {fp, ra} = {0, 0} sentinel at the bottom of pt_regs. + * fp/s0 points above the metadata record (RISC-V + * convention). + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) + li t0, FRAME_META_TYPE_FINAL + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + #if defined(CONFIG_RISCV_SBI) && defined(CONFIG_RISCV_USER_CFI) li a7, SBI_EXT_FWFT li a6, SBI_EXT_FWFT_SET diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index b2df7f72241a5..0dc90bf7a6524 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -258,8 +258,23 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) /* Supervisor/Machine, irqs on: */ childregs->status = SR_PP | SR_PIE; - p->thread.s[0] = (unsigned long)args->fn; - p->thread.s[1] = (unsigned long)args->fn_arg; + /* + * Set up a metadata frame record at the bottom of the + * stack for the unwinder. Use FRAME_META_TYPE_FINAL + * since this is the outermost kernel entry for the new + * task. The frame_record::{fp,ra} are already zero from + * memset(). + * + * fp/s0 points above the metadata record (RISC-V + * convention). fn and fn_arg are passed via s2/s3, + * keeping s0 available for the frame pointer chain. + */ + childregs->stackframe.type = FRAME_META_TYPE_FINAL; + + p->thread.s[0] = (unsigned long)(&childregs->stackframe) + + sizeof(struct frame_record); + p->thread.s[2] = (unsigned long)args->fn; + p->thread.s[3] = (unsigned long)args->fn_arg; p->thread.ra = (unsigned long)ret_from_fork_kernel_asm; } else { /* allocate new shadow stack if needed. In case of CLONE_VM we have to */ @@ -278,6 +293,20 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) if (clone_flags & CLONE_SETTLS) childregs->tp = tls; childregs->a0 = 0; /* Return value of fork() */ + + /* + * Set up the unwind boundary: ensure the metadata + * frame record has its {fp,ra} sentinel zeroed and + * point fp/s0 above the metadata record. Mark it as + * FINAL since this is the outermost kernel entry for + * the new task. + */ + childregs->stackframe.record.fp = 0; + childregs->stackframe.record.ra = 0; + childregs->stackframe.type = FRAME_META_TYPE_FINAL; + p->thread.s[0] = (unsigned long)(&childregs->stackframe) + + sizeof(struct frame_record); + p->thread.ra = (unsigned long)ret_from_fork_user_asm; } p->thread.riscv_v_flags = 0; -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata 2026-09-14 9:26 ` [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata Shuai Xue @ 2026-09-14 9:40 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:40 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > Reliable frame-pointer unwinding needs an explicit way to identify > exception boundaries and the final entry frame. The existing unwinder > infers those boundaries from return addresses, which is too loose for a > future reliable unwinder. > > Add a small metadata frame record to pt_regs and initialize it on > exception entry, kernel stack overflow, kernel thread fork, user fork, > and early idle task setup. The record uses a zero {fp, ra} sentinel plus > a type field so a later unwinder can distinguish a final user-to-kernel > boundary from a nested kernel pt_regs boundary. > > This follows the arm64 metadata frame-record model, adapted to the > RISC-V {fp, ra} frame record convention. > [ ... ] > > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=1 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue 2026-09-14 9:26 ` [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:32 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue ` (6 subsequent siblings) 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> KASAN records stack traces for every alloc/free, which means it walks the unwinder very frequently. Instrumenting the stack trace collection code itself adds substantial overhead and makes the traces themselves noisier. KCOV instruments every basic-block edge. The unwinder is a hot path, especially with KASAN enabled, so KCOV instrumentation has the same kind of cost and noise problem here. Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86 treatment of its stack unwinding code. RISC-V keeps the relevant unwinder code in stacktrace.o, so a single translation-unit annotation covers the equivalent scope. This is a prerequisite preference for the upcoming reliable unwinder, but the change is valid on its own. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index ebe1c3588177b..bf4645bb52403 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -44,6 +44,12 @@ CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) endif +# When KASAN is enabled, a stack trace is recorded for every alloc/free, which +# can significantly impact performance. Avoid instrumenting the stack trace +# collection code to minimize this impact. +KASAN_SANITIZE_stacktrace.o := n +KCOV_INSTRUMENT_stacktrace.o := n + always-$(KBUILD_BUILTIN) += vmlinux.lds obj-y += head.o -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o 2026-09-14 9:26 ` [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue @ 2026-09-14 9:32 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:32 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-perf-users, linux-trace-kernel > From: Wang Han <wanghan@linux.alibaba.com> > > KASAN records stack traces for every alloc/free, which means it walks > the unwinder very frequently. Instrumenting the stack trace collection > code itself adds substantial overhead and makes the traces themselves > noisier. > > KCOV instruments every basic-block edge. The unwinder is a hot path, > especially with KASAN enabled, so KCOV instrumentation has the same kind > of cost and noise problem here. > > Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86 > treatment of its stack unwinding code. RISC-V keeps the relevant unwinder > code in stacktrace.o, so a single translation-unit annotation covers the > equivalent scope. This is a prerequisite preference for the upcoming > reliable unwinder, but the change is valid on its own. > > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=2 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue 2026-09-14 9:26 ` [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata Shuai Xue 2026-09-14 9:26 ` [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:32 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue ` (5 subsequent siblings) 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> struct __arch_ftrace_regs declares s0 unconditionally, and both ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST (CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before, so any callback going through ftrace_partial_regs() saw a garbage regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this has not bitten in practice. Save and restore s0 unconditionally in the dynamic ftrace ABI register frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace path in line with the static _mcount path (mcount.S SAVE_ABI_STATE already saves s0 unconditionally), and matches the frame layout already documented in the comment above SAVE_ABI_REGS. It is also a prerequisite for the upcoming reliable unwinder, which reads ftrace_regs_get_frame_pointer(fregs) directly. The cost is one extra REG_S/REG_L pair per traced call, negligible compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK already reserved the slot, so no extra stack space is used. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/mcount-dyn.S | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index 082fe0b0e3c08..26c55fba8fec7 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -85,9 +85,7 @@ addi sp, sp, -FREGS_SIZE_ON_STACK REG_S t0, FREGS_EPC(sp) REG_S x1, FREGS_RA(sp) -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST REG_S x8, FREGS_S0(sp) -#endif REG_S x6, FREGS_T1(sp) #ifdef CONFIG_CC_IS_CLANG REG_S x7, FREGS_T2(sp) @@ -113,9 +111,7 @@ .macro RESTORE_ABI_REGS REG_L t0, FREGS_EPC(sp) REG_L x1, FREGS_RA(sp) -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST REG_L x8, FREGS_S0(sp) -#endif REG_L x6, FREGS_T1(sp) #ifdef CONFIG_CC_IS_CLANG REG_L x7, FREGS_T2(sp) -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame 2026-09-14 9:26 ` [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue @ 2026-09-14 9:32 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:32 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > struct __arch_ftrace_regs declares s0 unconditionally, and both > ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it > unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in > mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST > (CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With > CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before, > so any callback going through ftrace_partial_regs() saw a garbage > regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this > has not bitten in practice. > > Save and restore s0 unconditionally in the dynamic ftrace ABI register > frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace > path in line with the static _mcount path (mcount.S SAVE_ABI_STATE > already saves s0 unconditionally), and matches the frame layout already > documented in the comment above SAVE_ABI_REGS. It is also a prerequisite > for the upcoming reliable unwinder, which reads > ftrace_regs_get_frame_pointer(fregs) directly. > > The cost is one extra REG_S/REG_L pair per traced call, negligible > compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK > already reserved the slot, so no extra stack space is used. > > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=3 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (2 preceding siblings ...) 2026-09-14 9:26 ` [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:33 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue ` (4 subsequent siblings) 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> A reliable unwinder needs to validate that every frame record it reads is fully contained in a known kernel stack, and it needs to refuse to walk back into a stack it has already left. Add the building blocks for that: * struct stack_info / struct unwind_state in a new asm/stacktrace/common.h, modelled on the arm64 reference implementation. * stackinfo_get_irq() / stackinfo_get_task() / stackinfo_get_overflow() plus the corresponding on_*_stack() predicates in asm/stacktrace.h, so callers can ask "is this object on stack X?" by stack kind rather than open-coded address arithmetic. * unwind_init_common(), unwind_find_stack() and unwind_consume_stack() helpers that enforce the forward-progress-only invariant required for reliability. No existing user is wired up to these helpers in this commit; the unwinder switch comes in a follow-up. The header changes leave on_thread_stack() with the same semantics as before, just expressed in terms of the new helpers. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/include/asm/stacktrace.h | 67 ++++++++- arch/riscv/include/asm/stacktrace/common.h | 159 +++++++++++++++++++++ 2 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/common.h diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h index b1495a7e06ce6..df4d022f64304 100644 --- a/arch/riscv/include/asm/stacktrace.h +++ b/arch/riscv/include/asm/stacktrace.h @@ -3,8 +3,13 @@ #ifndef _ASM_RISCV_STACKTRACE_H #define _ASM_RISCV_STACKTRACE_H +#include <linux/percpu.h> #include <linux/sched.h> +#include <linux/sched/task_stack.h> + +#include <asm/irq_stack.h> #include <asm/ptrace.h> +#include <asm/stacktrace/common.h> struct stackframe { unsigned long fp; @@ -16,14 +21,72 @@ extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *re extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl); -static inline bool on_thread_stack(void) +/* + * IRQ stack accessors + */ +#ifdef CONFIG_IRQ_STACKS +static inline struct stack_info stackinfo_get_irq(void) +{ + unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); + unsigned long high = low + IRQ_STACK_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} + +static inline bool on_irq_stack(unsigned long sp, unsigned long size) +{ + struct stack_info info = stackinfo_get_irq(); + + return stackinfo_on_stack(&info, sp, size); +} +#endif /* CONFIG_IRQ_STACKS */ + +/* + * Task stack accessors + */ +static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk) { - return !(((unsigned long)(current->stack) ^ current_stack_pointer) & ~(THREAD_SIZE - 1)); + unsigned long low = (unsigned long)task_stack_page(tsk); + unsigned long high = low + THREAD_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} + +static inline bool on_task_stack(const struct task_struct *tsk, + unsigned long sp, unsigned long size) +{ + struct stack_info info = stackinfo_get_task(tsk); + + return stackinfo_on_stack(&info, sp, size); } +/* + * Cast is necessary since current->stack is an opaque ptr. + */ +#define on_thread_stack() (on_task_stack(current, current_stack_pointer, 1)) +/* + * Overflow stack accessors + */ #ifdef CONFIG_VMAP_STACK DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack); + +static inline struct stack_info stackinfo_get_overflow(void) +{ + unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack); + unsigned long high = low + OVERFLOW_STACK_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} #endif /* CONFIG_VMAP_STACK */ #endif /* _ASM_RISCV_STACKTRACE_H */ diff --git a/arch/riscv/include/asm/stacktrace/common.h b/arch/riscv/include/asm/stacktrace/common.h new file mode 100644 index 0000000000000..360a26e34349c --- /dev/null +++ b/arch/riscv/include/asm/stacktrace/common.h @@ -0,0 +1,159 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * RISC-V common stack unwinder types and helpers. + * + * See: arch/arm64/include/asm/stacktrace/common.h for the reference + * implementation. + * + * Copyright (C) 2026 + */ +#ifndef __ASM_RISCV_STACKTRACE_COMMON_H +#define __ASM_RISCV_STACKTRACE_COMMON_H + +#include <linux/compiler.h> +#include <linux/errno.h> +#include <linux/types.h> + +#include <asm/stacktrace/frame.h> + +/** + * struct stack_info - describes the bounds of a stack. + * + * @low: The lowest valid address on the stack. + * @high: The highest valid address on the stack. + */ +struct stack_info { + unsigned long low; + unsigned long high; +}; + +/** + * struct unwind_state - state used for robust unwinding. + * + * @fp: The fp value in the frame record (or the real fp). + * @pc: The ra value in the frame record (or the real ra). + * + * @stack: The stack currently being unwound. + * @stacks: An array of stacks which can be unwound. + * @nr_stacks: The number of stacks in @stacks. + */ +struct unwind_state { + unsigned long fp; + unsigned long pc; + + struct stack_info stack; + struct stack_info *stacks; + int nr_stacks; +}; + +/** + * stackinfo_get_unknown() - Get an unknown stack_info. + * + * Return: a stack_info with low and high set to 0. + */ +static inline struct stack_info stackinfo_get_unknown(void) +{ + return (struct stack_info) { + .low = 0, + .high = 0, + }; +} + +/** + * stackinfo_on_stack() - Check whether an object is fully within a stack. + * + * @info: The stack to check against. + * @sp: The base address of the object. + * @size: The size of the object. + * + * Return: true if the object is fully contained within the stack. + */ +static inline bool stackinfo_on_stack(const struct stack_info *info, + unsigned long sp, unsigned long size) +{ + if (!info->low) + return false; + + if (sp < info->low || sp + size < sp || sp + size > info->high) + return false; + + return true; +} + +/** + * unwind_init_common() - Initialize the common parts of the unwind state. + * + * @state: the unwind state to initialize. + */ +static inline void unwind_init_common(struct unwind_state *state) +{ + state->stack = stackinfo_get_unknown(); +} + +/** + * unwind_find_stack() - Find the accessible stack which entirely contains an + * object. + * + * @state: the current unwind state. + * @sp: the base address of the object. + * @size: the size of the object. + * + * Return: a pointer to the relevant stack_info if found; NULL otherwise. + */ +static inline struct stack_info *unwind_find_stack(struct unwind_state *state, + unsigned long sp, + unsigned long size) +{ + struct stack_info *info = &state->stack; + + if (stackinfo_on_stack(info, sp, size)) + return info; + + for (int i = 0; i < state->nr_stacks; i++) { + info = &state->stacks[i]; + if (stackinfo_on_stack(info, sp, size)) + return info; + } + + return NULL; +} + +/** + * unwind_consume_stack() - Update stack boundaries so that future unwind steps + * cannot consume this object again. + * + * @state: the current unwind state. + * @info: the stack_info of the stack containing the object. + * @sp: the base address of the object. + * @size: the size of the object. + * + * Stack transitions are strictly one-way, and once we've + * transitioned from one stack to another, it's never valid to + * unwind back to the old stack. + * + * Note that stacks can nest in several valid orders, e.g. + * + * TASK -> IRQ -> OVERFLOW + * + * ... so we do not check the specific order of stack + * transitions. + */ +static inline void unwind_consume_stack(struct unwind_state *state, + struct stack_info *info, + unsigned long sp, + unsigned long size) +{ + struct stack_info tmp; + + tmp = *info; + *info = stackinfo_get_unknown(); + state->stack = tmp; + + /* + * Future unwind steps can only consume stack above this frame record. + * Update the current stack to start immediately above it. + */ + state->stack.low = sp + size; +} + +#endif /* __ASM_RISCV_STACKTRACE_COMMON_H */ -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers 2026-09-14 9:26 ` [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue @ 2026-09-14 9:33 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:33 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > A reliable unwinder needs to validate that every frame record it reads > is fully contained in a known kernel stack, and it needs to refuse to > walk back into a stack it has already left. Add the building blocks > for that: > > * struct stack_info / struct unwind_state in a new > asm/stacktrace/common.h, modelled on the arm64 reference > implementation. > * stackinfo_get_irq() / stackinfo_get_task() / stackinfo_get_overflow() > plus the corresponding on_*_stack() predicates in asm/stacktrace.h, > so callers can ask "is this object on stack X?" by stack kind > rather than open-coded address arithmetic. > * unwind_init_common(), unwind_find_stack() and > unwind_consume_stack() helpers that enforce the > forward-progress-only invariant required for reliability. > > No existing user is wired up to these helpers in this commit; the > unwinder switch comes in a follow-up. The header changes leave > on_thread_stack() with the same semantics as before, just expressed in > terms of the new helpers. > > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=4 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (3 preceding siblings ...) 2026-09-14 9:26 ` [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:39 ` sashiko-bot ` (2 more replies) 2026-09-14 9:26 ` [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue ` (3 subsequent siblings) 8 siblings, 3 replies; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Replace the open-coded frame-pointer walker in arch_stack_walk() with a robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c and retargeted to the RISC-V {fp, ra} frame record convention. The new walker tracks stack bounds, consumes frame records monotonically, understands the metadata pt_regs records added in the previous frame record metadata patch, and recovers return addresses replaced by function graph tracing and kretprobes. This commit introduces arch_stack_walk_reliable() but does not yet select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig patch so this commit can be reviewed and bisected as a pure unwinder replacement. Until that Kconfig change lands, livepatch is not yet enabled and arch_stack_walk_reliable() has no in-tree caller. Three related callers are updated to keep the same frame-record assumptions everywhere: * Function graph tracing: the old RISC-V unwinder matched function graph return-stack entries by the saved return-address slot. That was consistent with the static mcount path, but not with the dynamic ftrace path where the parent slot is ftrace_regs::ra. Use the architectural frame pointer as the function graph return-address cookie, matching the kunwind walker. * Perf callchains: route kernel callchain collection through arch_stack_walk() so perf sees the same frame-pointer unwind behaviour as dump_stack() and the upcoming livepatch path. * dump_backtrace() / __get_wchan() / show_stack(): these now go through arch_stack_walk(); the explicit "Call Trace:" header is moved into dump_backtrace() to preserve the original output. The non-frame-pointer fallback walker is kept untouched for !CONFIG_FRAME_POINTER builds. Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/ftrace.c | 6 +- arch/riscv/kernel/perf_callchain.c | 2 +- arch/riscv/kernel/stacktrace.c | 561 ++++++++++++++++++++++++----- 3 files changed, 473 insertions(+), 96 deletions(-) diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index be8b685144174..0facd824c911d 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/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c index b465bc9eb870e..436af96ea59ca 100644 --- a/arch/riscv/kernel/perf_callchain.c +++ b/arch/riscv/kernel/perf_callchain.c @@ -44,5 +44,5 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, return; } - walk_stackframe(NULL, regs, fill_callchain, entry); + arch_stack_walk(fill_callchain, entry, NULL, regs); } diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c index c7555447149bf..0ce0cf0da6060 100644 --- a/arch/riscv/kernel/stacktrace.c +++ b/arch/riscv/kernel/stacktrace.c @@ -11,98 +11,16 @@ #include <linux/sched/task_stack.h> #include <linux/stacktrace.h> #include <linux/ftrace.h> +#include <linux/kprobes.h> +#include <linux/llist.h> #include <asm/stacktrace.h> -#ifdef CONFIG_FRAME_POINTER - /* - * This disables KASAN checking when reading a value from another task's stack, - * since the other task could be running on another CPU and could have poisoned - * the stack in the meantime. + * Non-frame-pointer fallback unwinder. + * Only compiled when CONFIG_FRAME_POINTER is not enabled. */ -#define READ_ONCE_TASK_STACK(task, x) \ -({ \ - unsigned long val; \ - unsigned long addr = x; \ - if ((task) == current) \ - val = READ_ONCE(addr); \ - else \ - val = READ_ONCE_NOCHECK(addr); \ - val; \ -}) - -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) -{ - unsigned long low, high; - - low = sp + sizeof(struct stackframe); - high = ALIGN(sp, THREAD_SIZE); - - return !(fp < low || fp > high || fp & 0x07); -} - -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg) -{ - unsigned long fp, sp, pc; - int graph_idx = 0; - int level = 0; - - if (regs) { - fp = frame_pointer(regs); - sp = user_stack_pointer(regs); - pc = instruction_pointer(regs); - } else if (task == NULL || task == current) { - fp = (unsigned long)__builtin_frame_address(0); - sp = current_stack_pointer; - pc = (unsigned long)walk_stackframe; - level = -1; - } else { - /* task blocked in __switch_to */ - fp = task->thread.s[0]; - sp = task->thread.sp; - pc = task->thread.ra; - } - - for (;;) { - struct stackframe *frame; - - if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc)))) - break; - - if (unlikely(!fp_is_valid(fp, sp))) - break; - - /* Unwind stack frame */ - frame = (struct stackframe *)fp - 1; - sp = fp; - if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp)) { - /* We hit function where ra is not saved on the stack */ - fp = frame->ra; - pc = regs->ra; - } else { - fp = READ_ONCE_TASK_STACK(task, frame->fp); - pc = READ_ONCE_TASK_STACK(task, frame->ra); - pc = ftrace_graph_ret_addr(task, &graph_idx, pc, - &frame->ra); - if (pc >= (unsigned long)handle_exception && - pc < (unsigned long)&ret_from_exception_end) { - if (unlikely(!fn(arg, pc))) - break; - - pc = ((struct pt_regs *)sp)->epc; - fp = ((struct pt_regs *)sp)->s0; - } - } - - } -} - -#else /* !CONFIG_FRAME_POINTER */ +#ifndef CONFIG_FRAME_POINTER void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) @@ -133,7 +51,12 @@ void notrace walk_stackframe(struct task_struct *task, } } -#endif /* CONFIG_FRAME_POINTER */ +#endif /* !CONFIG_FRAME_POINTER */ + +/* + * Common trace helpers. + * These are used by both the FP (kunwind) and non-FP (walk_stackframe) paths. + */ static bool print_trace_address(void *arg, unsigned long pc) { @@ -146,12 +69,12 @@ static bool print_trace_address(void *arg, unsigned long pc) noinline void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl) { - walk_stackframe(task, regs, print_trace_address, (void *)loglvl); + printk("%sCall Trace:\n", loglvl); + arch_stack_walk(print_trace_address, (void *)loglvl, task, regs); } void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl) { - pr_cont("%sCall Trace:\n", loglvl); dump_backtrace(NULL, task, loglvl); } @@ -171,17 +94,469 @@ unsigned long __get_wchan(struct task_struct *task) if (!try_get_task_stack(task)) return 0; - walk_stackframe(task, NULL, save_wchan, &pc); + arch_stack_walk(save_wchan, &pc, task, NULL); put_task_stack(task); return pc; } -noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, - struct task_struct *task, struct pt_regs *regs) +/* + * Frame-pointer-based kernel unwind infrastructure. + * Only compiled when CONFIG_FRAME_POINTER is enabled. + * + * See: arch/arm64/kernel/stacktrace.c for the reference implementation. + */ +#ifdef CONFIG_FRAME_POINTER + +/* + * Per-cpu stacks are only accessible when unwinding the current task in a + * non-preemptible context. + */ +#define STACKINFO_CPU(task, name) \ + ({ \ + (((task) == current) && !preemptible()) \ + ? stackinfo_get_##name() \ + : stackinfo_get_unknown(); \ + }) + +enum kunwind_source { + KUNWIND_SOURCE_UNKNOWN, + KUNWIND_SOURCE_FRAME, + KUNWIND_SOURCE_CALLER, + KUNWIND_SOURCE_TASK, + KUNWIND_SOURCE_REGS_PC, +}; + +union unwind_flags { + unsigned long all; + struct { + unsigned long fgraph : 1, + kretprobe : 1; + }; +}; + +/* + * Kernel unwind state + * + * @common: Common unwind state. + * @task: The task being unwound. + * @graph_idx: Used by ftrace_graph_ret_addr() for optimized stack unwinding. + * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance + * associated with the most recently encountered replacement ra + * value. + */ +struct kunwind_state { + struct unwind_state common; + struct task_struct *task; + int graph_idx; +#ifdef CONFIG_KRETPROBES + struct llist_node *kr_cur; +#endif + enum kunwind_source source; + union unwind_flags flags; + struct pt_regs *regs; +}; + +static __always_inline void +kunwind_init(struct kunwind_state *state, + struct task_struct *task) +{ + unwind_init_common(&state->common); + state->task = task; + state->source = KUNWIND_SOURCE_UNKNOWN; + state->flags.all = 0; + state->regs = NULL; +} + +/* + * Start an unwind from a pt_regs. + * + * The unwind will begin at the PC within the regs. + * + * The regs must be on a stack currently owned by the calling task. + */ +static __always_inline void +kunwind_init_from_regs(struct kunwind_state *state, + struct pt_regs *regs) +{ + kunwind_init(state, current); + + state->regs = regs; + state->common.fp = frame_pointer(regs); + state->common.pc = instruction_pointer(regs); + state->source = KUNWIND_SOURCE_REGS_PC; +} + +/* + * Start an unwind from a caller. + * + * The unwind will begin at the caller of whichever function this is inlined + * into. + * + * The function which invokes this must be noinline. + */ +static __always_inline void +kunwind_init_from_caller(struct kunwind_state *state) +{ + unsigned long fp = (unsigned long)__builtin_frame_address(0); + struct frame_record *record = (struct frame_record *)fp - 1; + + kunwind_init(state, current); + + state->common.fp = READ_ONCE(record->fp); + state->common.pc = READ_ONCE(record->ra); + state->source = KUNWIND_SOURCE_CALLER; +} + +/* + * Start an unwind from a blocked task. + * + * The unwind will begin at the blocked task's saved PC (i.e. the caller of + * __switch_to). + * + * The caller should ensure the task is blocked in __switch_to for the + * duration of the unwind, or the unwind will be bogus. It is never valid to + * call this for the current task. + */ +static __always_inline void +kunwind_init_from_task(struct kunwind_state *state, + struct task_struct *task) +{ + kunwind_init(state, task); + + state->common.fp = task->thread.s[0]; + state->common.pc = task->thread.ra; + state->source = KUNWIND_SOURCE_TASK; +} + +static __always_inline int +kunwind_recover_return_address(struct kunwind_state *state) +{ +#ifdef CONFIG_FUNCTION_GRAPH_TRACER + if (state->task->ret_stack && + state->common.pc == (unsigned long)return_to_handler) { + unsigned long orig_pc; + + orig_pc = ftrace_graph_ret_addr(state->task, &state->graph_idx, + state->common.pc, + (void *)state->common.fp); + if (state->common.pc == orig_pc) { + WARN_ON_ONCE(state->task == current); + return -EINVAL; + } + state->common.pc = orig_pc; + state->flags.fgraph = 1; + } +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ + +#ifdef CONFIG_KRETPROBES + if (is_kretprobe_trampoline(state->common.pc)) { + unsigned long orig_pc; + + orig_pc = kretprobe_find_ret_addr(state->task, + (void *)state->common.fp, + &state->kr_cur); + if (!orig_pc) + return -EINVAL; + state->common.pc = orig_pc; + state->flags.kretprobe = 1; + } +#endif /* CONFIG_KRETPROBES */ + + return 0; +} + +/* + * When we reach an exception boundary marked by a metadata frame record, + * extract pt_regs from the stack and continue unwinding from the saved + * context (epc and s0/fp). + * + * On RISC-V, fp points above the metadata record, so the record's + * frame_record portion is at fp - sizeof(struct frame_record). + */ +static __always_inline int +kunwind_next_regs_pc(struct kunwind_state *state) +{ + struct stack_info *info; + unsigned long fp = state->common.fp; + struct pt_regs *regs; + + regs = container_of((unsigned long *)(fp - sizeof(struct frame_record)), + struct pt_regs, stackframe.record.fp); + + info = unwind_find_stack(&state->common, (unsigned long)regs, + sizeof(*regs)); + if (!info) + return -EINVAL; + + unwind_consume_stack(&state->common, info, (unsigned long)regs, + sizeof(*regs)); + + state->regs = regs; + state->common.pc = regs->epc; + state->common.fp = frame_pointer(regs); + state->source = KUNWIND_SOURCE_REGS_PC; + return 0; +} + +/* + * Handle a metadata frame record embedded in pt_regs. + * + * On RISC-V, fp points above the record (fp = metadata + 16), so the + * frame_record_meta starts at fp - sizeof(struct frame_record). + * + * FRAME_META_TYPE_FINAL: This is the outermost exception entry + * (user -> kernel). Unwinding terminates successfully. + * FRAME_META_TYPE_PT_REGS: This is a nested exception entry + * (kernel -> kernel). Continue unwinding from the saved context. + */ +static __always_inline int +kunwind_next_frame_record_meta(struct kunwind_state *state) +{ + struct task_struct *tsk = state->task; + unsigned long fp = state->common.fp; + unsigned long meta_base = fp - sizeof(struct frame_record); + struct frame_record_meta *meta; + struct stack_info *info; + + info = unwind_find_stack(&state->common, meta_base, sizeof(*meta)); + if (!info) + return -EINVAL; + + meta = (struct frame_record_meta *)meta_base; + switch (READ_ONCE(meta->type)) { + case FRAME_META_TYPE_FINAL: + if (meta == &task_pt_regs(tsk)->stackframe) + return -ENOENT; + WARN_ON_ONCE(tsk == current); + return -EINVAL; + case FRAME_META_TYPE_PT_REGS: + return kunwind_next_regs_pc(state); + default: + WARN_ON_ONCE(tsk == current); + return -EINVAL; + } +} + +/* + * Unwind from one frame record to the next. + * + * On RISC-V, the frame record sits at fp - sizeof(struct frame_record), + * immediately below the address pointed to by fp/s0. This applies to both + * normal frame records and metadata frame records (embedded in pt_regs). + * + * A metadata record is identified by both fp and ra being zero in the + * frame_record portion, with a type value following at fp + 16. + */ +static __always_inline int +kunwind_next_frame_record(struct kunwind_state *state) +{ + unsigned long fp = state->common.fp; + struct frame_record *record; + struct stack_info *info; + unsigned long new_fp, new_pc; + unsigned long record_base; + + if (fp & 0x7) + return -EINVAL; + + record_base = fp - sizeof(*record); + + info = unwind_find_stack(&state->common, record_base, sizeof(*record)); + if (!info) + return -EINVAL; + + record = (struct frame_record *)record_base; + new_fp = READ_ONCE(record->fp); + new_pc = READ_ONCE(record->ra); + + if (!new_fp && !new_pc) + return kunwind_next_frame_record_meta(state); + + unwind_consume_stack(&state->common, info, record_base, + sizeof(*record)); + + state->common.fp = new_fp; + state->common.pc = new_pc; + state->source = KUNWIND_SOURCE_FRAME; + + return 0; +} + +/* + * Unwind from one frame record (A) to the next frame record (B). + * + * We terminate early if the location of B indicates a malformed chain of frame + * records (e.g. a cycle), determined based on the location and fp value of A + * and the location (but not the fp value) of B. + */ +static __always_inline int +kunwind_next(struct kunwind_state *state) +{ + int err; + + state->flags.all = 0; + + switch (state->source) { + case KUNWIND_SOURCE_FRAME: + case KUNWIND_SOURCE_CALLER: + case KUNWIND_SOURCE_TASK: + case KUNWIND_SOURCE_REGS_PC: + err = kunwind_next_frame_record(state); + break; + default: + err = -EINVAL; + } + + if (err) + return err; + + return kunwind_recover_return_address(state); +} + +typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie); + +static __always_inline int +do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state, + void *cookie) +{ + int ret; + + ret = kunwind_recover_return_address(state); + if (ret) + return ret; + + while (1) { + if (!consume_state(state, cookie)) + return -EINVAL; + ret = kunwind_next(state); + if (ret == -ENOENT) + return 0; + if (ret < 0) + return ret; + } +} + +static __always_inline int +kunwind_stack_walk(kunwind_consume_fn consume_state, + void *cookie, struct task_struct *task, + struct pt_regs *regs) +{ + struct task_struct *tsk = task ?: current; + struct stack_info stacks[] = { + stackinfo_get_task(tsk), +#ifdef CONFIG_IRQ_STACKS + STACKINFO_CPU(tsk, irq), +#endif +#ifdef CONFIG_VMAP_STACK + STACKINFO_CPU(tsk, overflow), +#endif + }; + struct kunwind_state state = { + .common = { + .stacks = stacks, + .nr_stacks = ARRAY_SIZE(stacks), + }, + }; + + if (regs) { + if (tsk != current) + return -EINVAL; + kunwind_init_from_regs(&state, regs); + } else if (tsk == current) { + kunwind_init_from_caller(&state); + } else { + kunwind_init_from_task(&state, tsk); + } + + return do_kunwind(&state, consume_state, cookie); +} + +struct kunwind_consume_entry_data { + stack_trace_consume_fn consume_entry; + void *cookie; +}; + +static __always_inline bool +arch_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) +{ + struct kunwind_consume_entry_data *data = cookie; + + return data->consume_entry(data->cookie, state->common.pc); +} + +static __always_inline bool +arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) +{ + /* + * At an exception boundary we can reliably consume the saved PC. We do + * not know whether ra was live when the exception was taken, and + * so we cannot perform the next unwind step reliably. + * + * All that matters is whether the *entire* unwind is reliable, so give + * up as soon as we hit an exception boundary. + */ + if (state->source == KUNWIND_SOURCE_REGS_PC) + return false; + + return arch_kunwind_consume_entry(state, cookie); +} + +#endif /* CONFIG_FRAME_POINTER */ + +/* + * arch_stack_walk - dual implementation. + * + * When CONFIG_FRAME_POINTER is enabled, uses the kunwind infrastructure for + * robust frame-pointer-based unwinding, consistent with arch_stack_walk_reliable. + * + * When CONFIG_FRAME_POINTER is disabled, falls back to the simple stack scan + * in walk_stackframe(). + */ +#ifdef CONFIG_FRAME_POINTER + +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) +{ + struct kunwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); +} + +#else + +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) { walk_stackframe(task, regs, consume_entry, cookie); } +#endif /* CONFIG_FRAME_POINTER */ + +/* + * Reliable stack walk for livepatch (CONFIG_FRAME_POINTER only). + */ +#ifdef CONFIG_FRAME_POINTER + +noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, + void *cookie, + struct task_struct *task) +{ + struct kunwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + return kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data, + task, NULL); +} + +#endif /* CONFIG_FRAME_POINTER */ + /* * Get the return address for a single stackframe and return a pointer to the * next frame tail. -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue @ 2026-09-14 9:39 ` sashiko-bot 2026-10-09 18:59 ` Karl Mehltretter 2026-10-13 20:18 ` Martin Kaiser 2 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:39 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > Replace the open-coded frame-pointer walker in arch_stack_walk() with a > robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c > and retargeted to the RISC-V {fp, ra} frame record convention. The new > walker tracks stack bounds, consumes frame records monotonically, > understands the metadata pt_regs records added in the previous frame > record metadata patch, and recovers return addresses replaced by > function graph tracing and kretprobes. > > This commit introduces arch_stack_walk_reliable() but does not yet > select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig > patch so this commit can be reviewed and bisected as a pure unwinder > replacement. Until that Kconfig change lands, livepatch is not yet > enabled and arch_stack_walk_reliable() has no in-tree caller. > [ ... ] > !CONFIG_FRAME_POINTER builds. > > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=5 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue 2026-09-14 9:39 ` sashiko-bot @ 2026-10-09 18:59 ` Karl Mehltretter 2026-10-10 3:38 ` Shuai Xue 2026-10-13 20:18 ` Martin Kaiser 2 siblings, 1 reply; 39+ messages in thread From: Karl Mehltretter @ 2026-10-09 18:59 UTC (permalink / raw) To: Shuai Xue Cc: Karl Mehltretter, palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, martin, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest On Mon, Sep 14, 2026 at 05:26:46PM +0800, Shuai Xue wrote: > +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > + void *cookie, struct task_struct *task, > + struct pt_regs *regs) > +{ > + struct kunwind_consume_entry_data data = { > + .consume_entry = consume_entry, > + .cookie = cookie, > + }; > + > + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); > +} Hi Shuai Xue, With this patch and FRAME_POINTER enabled, return_address() returns the caller one level further up than asked for. return_address() in arch/riscv/kernel/return_address.c skips level + 3 entries from arch_stack_walk(). The arm64 original skips level + 2. The third entry exists on mainline because the walk starts inside walk_stackframe() and first reports the return into arch_stack_walk(). With 5/7 the walk starts at the caller of arch_stack_walk(), so level + 3 now skips a real caller. CALLER_ADDR1 and up are built on return_address(). The irqsoff tracer and the preemptirq tracepoints use them. A KUnit test that calls ftrace_return_address() from leaf() <- middle() <- outer() <- check() returns: af32da41b032 with series with series, level + 2 level 0 middle outer middle level 1 outer check outer The change to level + 2 has to depend on FRAME_POINTER. Without it the series keeps walk_stackframe() and the result is the same as on mainline. There, level + 2 makes return_address(0) return an address inside return_address(). The same extra entry makes stack_trace_save() start one frame too early on mainline. 5/7 fixes that. I have a small fix for it on mainline, meant for stable kernels, which conflicts with 5/7. I would post it with you in Cc. Tell me if you would rather take the return_address.c change into 5/7 first. Tested with clang 22 and the series applied on 52318cf0fa6e. The "with series" column is the same on QEMU's virt machine and on a BeagleV Ahead (TH1520). Everything else ran on QEMU only. Karl ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-10-09 18:59 ` Karl Mehltretter @ 2026-10-10 3:38 ` Shuai Xue 0 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-10-10 3:38 UTC (permalink / raw) To: Karl Mehltretter Cc: palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, martin, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest On 10/10/26 2:59 AM, Karl Mehltretter wrote: > On Mon, Sep 14, 2026 at 05:26:46PM +0800, Shuai Xue wrote: >> +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, >> + void *cookie, struct task_struct *task, >> + struct pt_regs *regs) >> +{ >> + struct kunwind_consume_entry_data data = { >> + .consume_entry = consume_entry, >> + .cookie = cookie, >> + }; >> + >> + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); >> +} > > Hi Shuai Xue, > > With this patch and FRAME_POINTER enabled, return_address() returns the > caller one level further up than asked for. > > return_address() in arch/riscv/kernel/return_address.c skips level + 3 > entries from arch_stack_walk(). The arm64 original skips level + 2. The > third entry exists on mainline because the walk starts inside > walk_stackframe() and first reports the return into arch_stack_walk(). > With 5/7 the walk starts at the caller of arch_stack_walk(), so > level + 3 now skips a real caller. CALLER_ADDR1 and up are built on > return_address(). The irqsoff tracer and the preemptirq tracepoints use > them. > > A KUnit test that calls ftrace_return_address() from > leaf() <- middle() <- outer() <- check() returns: > > af32da41b032 with series with series, level + 2 > level 0 middle outer middle > level 1 outer check outer > > The change to level + 2 has to depend on FRAME_POINTER. Without it the > series keeps walk_stackframe() and the result is the same as on > mainline. There, level + 2 makes return_address(0) return an address > inside return_address(). Hi, Karl, Nice point. Thank you for the thorough review and the KUnit test. The analysis is spot on, and your table matches my reading of the code exactly. > > The same extra entry makes stack_trace_save() start one frame too early > on mainline. 5/7 fixes that. I have a small fix for it on mainline, > meant for stable kernels, which conflicts with 5/7. I would post it with > you in Cc. Tell me if you would rather take the return_address.c change > into 5/7 first. Please go ahead with your mainline/stable fix for the stack_trace_save() off-by-one as an independent patch. You are right that this is a separate mainline bug: stack_trace_save() hardcodes .skip = skipnr + 1, i.e. it expects the first entry reported by arch_stack_walk() to be the return into stack_trace_save() itself, and mainline's walk_stackframe() breaks that contract by reporting the return into arch_stack_walk() first. A 7-patch series is not a vehicle for stable, so the minimal fix is the right approach there. Please keep me in Cc when you post it, I will review and test it. > > Tested with clang 22 and the series applied on 52318cf0fa6e. The > "with series" column is the same on QEMU's virt machine and on a > BeagleV Ahead (TH1520). Everything else ran on QEMU only. Thanks again for testing on real hardware (BeagleV Ahead) in addition to QEMU, that is much appreciated. Best regards, Shuai Xue ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue 2026-09-14 9:39 ` sashiko-bot 2026-10-09 18:59 ` Karl Mehltretter @ 2026-10-13 20:18 ` Martin Kaiser 2026-10-14 13:09 ` Shuai Xue 2 siblings, 1 reply; 39+ messages in thread From: Martin Kaiser @ 2026-10-13 20:18 UTC (permalink / raw) To: Shuai Xue Cc: palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest Thus wrote Shuai Xue (xueshuai@linux.alibaba.com): > From: Wang Han <wanghan@linux.alibaba.com> > Replace the open-coded frame-pointer walker in arch_stack_walk() with a > robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c > and retargeted to the RISC-V {fp, ra} frame record convention. The new > walker tracks stack bounds, consumes frame records monotonically, > understands the metadata pt_regs records added in the previous frame > record metadata patch, and recovers return addresses replaced by > function graph tracing and kretprobes. > This commit introduces arch_stack_walk_reliable() but does not yet > select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig > patch so this commit can be reviewed and bisected as a pure unwinder > replacement. Until that Kconfig change lands, livepatch is not yet > enabled and arch_stack_walk_reliable() has no in-tree caller. > Three related callers are updated to keep the same frame-record > assumptions everywhere: > * Function graph tracing: the old RISC-V unwinder matched function > graph return-stack entries by the saved return-address slot. That > was consistent with the static mcount path, but not with the dynamic > ftrace path where the parent slot is ftrace_regs::ra. Use the > architectural frame pointer as the function graph return-address > cookie, matching the kunwind walker. > * Perf callchains: route kernel callchain collection through > arch_stack_walk() so perf sees the same frame-pointer unwind > behaviour as dump_stack() and the upcoming livepatch path. > * dump_backtrace() / __get_wchan() / show_stack(): these now go > through arch_stack_walk(); the explicit "Call Trace:" header is > moved into dump_backtrace() to preserve the original output. > The non-frame-pointer fallback walker is kept untouched for > !CONFIG_FRAME_POINTER builds. > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > --- > arch/riscv/kernel/ftrace.c | 6 +- > arch/riscv/kernel/perf_callchain.c | 2 +- > arch/riscv/kernel/stacktrace.c | 561 ++++++++++++++++++++++++----- > 3 files changed, 473 insertions(+), 96 deletions(-) > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c > index be8b685144174..0facd824c911d 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; > } Are these two changes fixing a bug in the current code (that is independent from the rest of this patch)? Without your changes, function_graph_enter_regs writes a struct ftrace_ret_stack with retp == the saved ra. And the only place where retp is fetched and checked is function_get_true_parent_ip ftrace_graph_ret_addr(..., (uintptr_t *)ftrace_regs_get_stack_pointer(fregs)); The stored retp is checked against the stack pointer. Your changes store fp instead of ra, this seems the right thing to do. (And it's in line with other archs.) Should those two changes be in a separate patch (that can be backported to stable kernels)? Thanks, Martin > #endif /* CONFIG_DYNAMIC_FTRACE */ > diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c > index b465bc9eb870e..436af96ea59ca 100644 > --- a/arch/riscv/kernel/perf_callchain.c > +++ b/arch/riscv/kernel/perf_callchain.c > @@ -44,5 +44,5 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, > return; > } > - walk_stackframe(NULL, regs, fill_callchain, entry); > + arch_stack_walk(fill_callchain, entry, NULL, regs); > } > diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c > index c7555447149bf..0ce0cf0da6060 100644 > --- a/arch/riscv/kernel/stacktrace.c > +++ b/arch/riscv/kernel/stacktrace.c > @@ -11,98 +11,16 @@ > #include <linux/sched/task_stack.h> > #include <linux/stacktrace.h> > #include <linux/ftrace.h> > +#include <linux/kprobes.h> > +#include <linux/llist.h> > #include <asm/stacktrace.h> > -#ifdef CONFIG_FRAME_POINTER > - > /* > - * This disables KASAN checking when reading a value from another task's stack, > - * since the other task could be running on another CPU and could have poisoned > - * the stack in the meantime. > + * Non-frame-pointer fallback unwinder. > + * Only compiled when CONFIG_FRAME_POINTER is not enabled. > */ > -#define READ_ONCE_TASK_STACK(task, x) \ > -({ \ > - unsigned long val; \ > - unsigned long addr = x; \ > - if ((task) == current) \ > - val = READ_ONCE(addr); \ > - else \ > - val = READ_ONCE_NOCHECK(addr); \ > - val; \ > -}) > - > -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) > -{ > - unsigned long low, high; > - > - low = sp + sizeof(struct stackframe); > - high = ALIGN(sp, THREAD_SIZE); > - > - return !(fp < low || fp > high || fp & 0x07); > -} > - > -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > - bool (*fn)(void *, unsigned long), void *arg) > -{ > - unsigned long fp, sp, pc; > - int graph_idx = 0; > - int level = 0; > - > - if (regs) { > - fp = frame_pointer(regs); > - sp = user_stack_pointer(regs); > - pc = instruction_pointer(regs); > - } else if (task == NULL || task == current) { > - fp = (unsigned long)__builtin_frame_address(0); > - sp = current_stack_pointer; > - pc = (unsigned long)walk_stackframe; > - level = -1; > - } else { > - /* task blocked in __switch_to */ > - fp = task->thread.s[0]; > - sp = task->thread.sp; > - pc = task->thread.ra; > - } > - > - for (;;) { > - struct stackframe *frame; > - > - if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc)))) > - break; > - > - if (unlikely(!fp_is_valid(fp, sp))) > - break; > - > - /* Unwind stack frame */ > - frame = (struct stackframe *)fp - 1; > - sp = fp; > - if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp)) { > - /* We hit function where ra is not saved on the stack */ > - fp = frame->ra; > - pc = regs->ra; > - } else { > - fp = READ_ONCE_TASK_STACK(task, frame->fp); > - pc = READ_ONCE_TASK_STACK(task, frame->ra); > - pc = ftrace_graph_ret_addr(task, &graph_idx, pc, > - &frame->ra); > - if (pc >= (unsigned long)handle_exception && > - pc < (unsigned long)&ret_from_exception_end) { > - if (unlikely(!fn(arg, pc))) > - break; > - > - pc = ((struct pt_regs *)sp)->epc; > - fp = ((struct pt_regs *)sp)->s0; > - } > - } > - > - } > -} > - > -#else /* !CONFIG_FRAME_POINTER */ > +#ifndef CONFIG_FRAME_POINTER > void notrace walk_stackframe(struct task_struct *task, > struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) > @@ -133,7 +51,12 @@ void notrace walk_stackframe(struct task_struct *task, > } > } > -#endif /* CONFIG_FRAME_POINTER */ > +#endif /* !CONFIG_FRAME_POINTER */ > + > +/* > + * Common trace helpers. > + * These are used by both the FP (kunwind) and non-FP (walk_stackframe) paths. > + */ > static bool print_trace_address(void *arg, unsigned long pc) > { > @@ -146,12 +69,12 @@ static bool print_trace_address(void *arg, unsigned long pc) > noinline void dump_backtrace(struct pt_regs *regs, struct task_struct *task, > const char *loglvl) > { > - walk_stackframe(task, regs, print_trace_address, (void *)loglvl); > + printk("%sCall Trace:\n", loglvl); > + arch_stack_walk(print_trace_address, (void *)loglvl, task, regs); > } > void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl) > { > - pr_cont("%sCall Trace:\n", loglvl); > dump_backtrace(NULL, task, loglvl); > } > @@ -171,17 +94,469 @@ unsigned long __get_wchan(struct task_struct *task) > if (!try_get_task_stack(task)) > return 0; > - walk_stackframe(task, NULL, save_wchan, &pc); > + arch_stack_walk(save_wchan, &pc, task, NULL); > put_task_stack(task); > return pc; > } > -noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > - struct task_struct *task, struct pt_regs *regs) > +/* > + * Frame-pointer-based kernel unwind infrastructure. > + * Only compiled when CONFIG_FRAME_POINTER is enabled. > + * > + * See: arch/arm64/kernel/stacktrace.c for the reference implementation. > + */ > +#ifdef CONFIG_FRAME_POINTER > + > +/* > + * Per-cpu stacks are only accessible when unwinding the current task in a > + * non-preemptible context. > + */ > +#define STACKINFO_CPU(task, name) \ > + ({ \ > + (((task) == current) && !preemptible()) \ > + ? stackinfo_get_##name() \ > + : stackinfo_get_unknown(); \ > + }) > + > +enum kunwind_source { > + KUNWIND_SOURCE_UNKNOWN, > + KUNWIND_SOURCE_FRAME, > + KUNWIND_SOURCE_CALLER, > + KUNWIND_SOURCE_TASK, > + KUNWIND_SOURCE_REGS_PC, > +}; > + > +union unwind_flags { > + unsigned long all; > + struct { > + unsigned long fgraph : 1, > + kretprobe : 1; > + }; > +}; > + > +/* > + * Kernel unwind state > + * > + * @common: Common unwind state. > + * @task: The task being unwound. > + * @graph_idx: Used by ftrace_graph_ret_addr() for optimized stack unwinding. > + * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance > + * associated with the most recently encountered replacement ra > + * value. > + */ > +struct kunwind_state { > + struct unwind_state common; > + struct task_struct *task; > + int graph_idx; > +#ifdef CONFIG_KRETPROBES > + struct llist_node *kr_cur; > +#endif > + enum kunwind_source source; > + union unwind_flags flags; > + struct pt_regs *regs; > +}; > + > +static __always_inline void > +kunwind_init(struct kunwind_state *state, > + struct task_struct *task) > +{ > + unwind_init_common(&state->common); > + state->task = task; > + state->source = KUNWIND_SOURCE_UNKNOWN; > + state->flags.all = 0; > + state->regs = NULL; > +} > + > +/* > + * Start an unwind from a pt_regs. > + * > + * The unwind will begin at the PC within the regs. > + * > + * The regs must be on a stack currently owned by the calling task. > + */ > +static __always_inline void > +kunwind_init_from_regs(struct kunwind_state *state, > + struct pt_regs *regs) > +{ > + kunwind_init(state, current); > + > + state->regs = regs; > + state->common.fp = frame_pointer(regs); > + state->common.pc = instruction_pointer(regs); > + state->source = KUNWIND_SOURCE_REGS_PC; > +} > + > +/* > + * Start an unwind from a caller. > + * > + * The unwind will begin at the caller of whichever function this is inlined > + * into. > + * > + * The function which invokes this must be noinline. > + */ > +static __always_inline void > +kunwind_init_from_caller(struct kunwind_state *state) > +{ > + unsigned long fp = (unsigned long)__builtin_frame_address(0); > + struct frame_record *record = (struct frame_record *)fp - 1; > + > + kunwind_init(state, current); > + > + state->common.fp = READ_ONCE(record->fp); > + state->common.pc = READ_ONCE(record->ra); > + state->source = KUNWIND_SOURCE_CALLER; > +} > + > +/* > + * Start an unwind from a blocked task. > + * > + * The unwind will begin at the blocked task's saved PC (i.e. the caller of > + * __switch_to). > + * > + * The caller should ensure the task is blocked in __switch_to for the > + * duration of the unwind, or the unwind will be bogus. It is never valid to > + * call this for the current task. > + */ > +static __always_inline void > +kunwind_init_from_task(struct kunwind_state *state, > + struct task_struct *task) > +{ > + kunwind_init(state, task); > + > + state->common.fp = task->thread.s[0]; > + state->common.pc = task->thread.ra; > + state->source = KUNWIND_SOURCE_TASK; > +} > + > +static __always_inline int > +kunwind_recover_return_address(struct kunwind_state *state) > +{ > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER > + if (state->task->ret_stack && > + state->common.pc == (unsigned long)return_to_handler) { > + unsigned long orig_pc; > + > + orig_pc = ftrace_graph_ret_addr(state->task, &state->graph_idx, > + state->common.pc, > + (void *)state->common.fp); > + if (state->common.pc == orig_pc) { > + WARN_ON_ONCE(state->task == current); > + return -EINVAL; > + } > + state->common.pc = orig_pc; > + state->flags.fgraph = 1; > + } > +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > + > +#ifdef CONFIG_KRETPROBES > + if (is_kretprobe_trampoline(state->common.pc)) { > + unsigned long orig_pc; > + > + orig_pc = kretprobe_find_ret_addr(state->task, > + (void *)state->common.fp, > + &state->kr_cur); > + if (!orig_pc) > + return -EINVAL; > + state->common.pc = orig_pc; > + state->flags.kretprobe = 1; > + } > +#endif /* CONFIG_KRETPROBES */ > + > + return 0; > +} > + > +/* > + * When we reach an exception boundary marked by a metadata frame record, > + * extract pt_regs from the stack and continue unwinding from the saved > + * context (epc and s0/fp). > + * > + * On RISC-V, fp points above the metadata record, so the record's > + * frame_record portion is at fp - sizeof(struct frame_record). > + */ > +static __always_inline int > +kunwind_next_regs_pc(struct kunwind_state *state) > +{ > + struct stack_info *info; > + unsigned long fp = state->common.fp; > + struct pt_regs *regs; > + > + regs = container_of((unsigned long *)(fp - sizeof(struct frame_record)), > + struct pt_regs, stackframe.record.fp); > + > + info = unwind_find_stack(&state->common, (unsigned long)regs, > + sizeof(*regs)); > + if (!info) > + return -EINVAL; > + > + unwind_consume_stack(&state->common, info, (unsigned long)regs, > + sizeof(*regs)); > + > + state->regs = regs; > + state->common.pc = regs->epc; > + state->common.fp = frame_pointer(regs); > + state->source = KUNWIND_SOURCE_REGS_PC; > + return 0; > +} > + > +/* > + * Handle a metadata frame record embedded in pt_regs. > + * > + * On RISC-V, fp points above the record (fp = metadata + 16), so the > + * frame_record_meta starts at fp - sizeof(struct frame_record). > + * > + * FRAME_META_TYPE_FINAL: This is the outermost exception entry > + * (user -> kernel). Unwinding terminates successfully. > + * FRAME_META_TYPE_PT_REGS: This is a nested exception entry > + * (kernel -> kernel). Continue unwinding from the saved context. > + */ > +static __always_inline int > +kunwind_next_frame_record_meta(struct kunwind_state *state) > +{ > + struct task_struct *tsk = state->task; > + unsigned long fp = state->common.fp; > + unsigned long meta_base = fp - sizeof(struct frame_record); > + struct frame_record_meta *meta; > + struct stack_info *info; > + > + info = unwind_find_stack(&state->common, meta_base, sizeof(*meta)); > + if (!info) > + return -EINVAL; > + > + meta = (struct frame_record_meta *)meta_base; > + switch (READ_ONCE(meta->type)) { > + case FRAME_META_TYPE_FINAL: > + if (meta == &task_pt_regs(tsk)->stackframe) > + return -ENOENT; > + WARN_ON_ONCE(tsk == current); > + return -EINVAL; > + case FRAME_META_TYPE_PT_REGS: > + return kunwind_next_regs_pc(state); > + default: > + WARN_ON_ONCE(tsk == current); > + return -EINVAL; > + } > +} > + > +/* > + * Unwind from one frame record to the next. > + * > + * On RISC-V, the frame record sits at fp - sizeof(struct frame_record), > + * immediately below the address pointed to by fp/s0. This applies to both > + * normal frame records and metadata frame records (embedded in pt_regs). > + * > + * A metadata record is identified by both fp and ra being zero in the > + * frame_record portion, with a type value following at fp + 16. > + */ > +static __always_inline int > +kunwind_next_frame_record(struct kunwind_state *state) > +{ > + unsigned long fp = state->common.fp; > + struct frame_record *record; > + struct stack_info *info; > + unsigned long new_fp, new_pc; > + unsigned long record_base; > + > + if (fp & 0x7) > + return -EINVAL; > + > + record_base = fp - sizeof(*record); > + > + info = unwind_find_stack(&state->common, record_base, sizeof(*record)); > + if (!info) > + return -EINVAL; > + > + record = (struct frame_record *)record_base; > + new_fp = READ_ONCE(record->fp); > + new_pc = READ_ONCE(record->ra); > + > + if (!new_fp && !new_pc) > + return kunwind_next_frame_record_meta(state); > + > + unwind_consume_stack(&state->common, info, record_base, > + sizeof(*record)); > + > + state->common.fp = new_fp; > + state->common.pc = new_pc; > + state->source = KUNWIND_SOURCE_FRAME; > + > + return 0; > +} > + > +/* > + * Unwind from one frame record (A) to the next frame record (B). > + * > + * We terminate early if the location of B indicates a malformed chain of frame > + * records (e.g. a cycle), determined based on the location and fp value of A > + * and the location (but not the fp value) of B. > + */ > +static __always_inline int > +kunwind_next(struct kunwind_state *state) > +{ > + int err; > + > + state->flags.all = 0; > + > + switch (state->source) { > + case KUNWIND_SOURCE_FRAME: > + case KUNWIND_SOURCE_CALLER: > + case KUNWIND_SOURCE_TASK: > + case KUNWIND_SOURCE_REGS_PC: > + err = kunwind_next_frame_record(state); > + break; > + default: > + err = -EINVAL; > + } > + > + if (err) > + return err; > + > + return kunwind_recover_return_address(state); > +} > + > +typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie); > + > +static __always_inline int > +do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state, > + void *cookie) > +{ > + int ret; > + > + ret = kunwind_recover_return_address(state); > + if (ret) > + return ret; > + > + while (1) { > + if (!consume_state(state, cookie)) > + return -EINVAL; > + ret = kunwind_next(state); > + if (ret == -ENOENT) > + return 0; > + if (ret < 0) > + return ret; > + } > +} > + > +static __always_inline int > +kunwind_stack_walk(kunwind_consume_fn consume_state, > + void *cookie, struct task_struct *task, > + struct pt_regs *regs) > +{ > + struct task_struct *tsk = task ?: current; > + struct stack_info stacks[] = { > + stackinfo_get_task(tsk), > +#ifdef CONFIG_IRQ_STACKS > + STACKINFO_CPU(tsk, irq), > +#endif > +#ifdef CONFIG_VMAP_STACK > + STACKINFO_CPU(tsk, overflow), > +#endif > + }; > + struct kunwind_state state = { > + .common = { > + .stacks = stacks, > + .nr_stacks = ARRAY_SIZE(stacks), > + }, > + }; > + > + if (regs) { > + if (tsk != current) > + return -EINVAL; > + kunwind_init_from_regs(&state, regs); > + } else if (tsk == current) { > + kunwind_init_from_caller(&state); > + } else { > + kunwind_init_from_task(&state, tsk); > + } > + > + return do_kunwind(&state, consume_state, cookie); > +} > + > +struct kunwind_consume_entry_data { > + stack_trace_consume_fn consume_entry; > + void *cookie; > +}; > + > +static __always_inline bool > +arch_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) > +{ > + struct kunwind_consume_entry_data *data = cookie; > + > + return data->consume_entry(data->cookie, state->common.pc); > +} > + > +static __always_inline bool > +arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) > +{ > + /* > + * At an exception boundary we can reliably consume the saved PC. We do > + * not know whether ra was live when the exception was taken, and > + * so we cannot perform the next unwind step reliably. > + * > + * All that matters is whether the *entire* unwind is reliable, so give > + * up as soon as we hit an exception boundary. > + */ > + if (state->source == KUNWIND_SOURCE_REGS_PC) > + return false; > + > + return arch_kunwind_consume_entry(state, cookie); > +} > + > +#endif /* CONFIG_FRAME_POINTER */ > + > +/* > + * arch_stack_walk - dual implementation. > + * > + * When CONFIG_FRAME_POINTER is enabled, uses the kunwind infrastructure for > + * robust frame-pointer-based unwinding, consistent with arch_stack_walk_reliable. > + * > + * When CONFIG_FRAME_POINTER is disabled, falls back to the simple stack scan > + * in walk_stackframe(). > + */ > +#ifdef CONFIG_FRAME_POINTER > + > +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > + void *cookie, struct task_struct *task, > + struct pt_regs *regs) > +{ > + struct kunwind_consume_entry_data data = { > + .consume_entry = consume_entry, > + .cookie = cookie, > + }; > + > + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); > +} > + > +#else > + > +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > + void *cookie, struct task_struct *task, > + struct pt_regs *regs) > { > walk_stackframe(task, regs, consume_entry, cookie); > } > +#endif /* CONFIG_FRAME_POINTER */ > + > +/* > + * Reliable stack walk for livepatch (CONFIG_FRAME_POINTER only). > + */ > +#ifdef CONFIG_FRAME_POINTER > + > +noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, > + void *cookie, > + struct task_struct *task) > +{ > + struct kunwind_consume_entry_data data = { > + .consume_entry = consume_entry, > + .cookie = cookie, > + }; > + > + return kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data, > + task, NULL); > +} > + > +#endif /* CONFIG_FRAME_POINTER */ > + > /* > * Get the return address for a single stackframe and return a pointer to the > * next frame tail. > -- > 2.39.3 > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder 2026-10-13 20:18 ` Martin Kaiser @ 2026-10-14 13:09 ` Shuai Xue 0 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-10-14 13:09 UTC (permalink / raw) To: Martin Kaiser Cc: palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest On 10/14/26 4:18 AM, Martin Kaiser wrote: > Thus wrote Shuai Xue (xueshuai@linux.alibaba.com): > >> From: Wang Han <wanghan@linux.alibaba.com> > >> Replace the open-coded frame-pointer walker in arch_stack_walk() with a >> robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c >> and retargeted to the RISC-V {fp, ra} frame record convention. The new >> walker tracks stack bounds, consumes frame records monotonically, >> understands the metadata pt_regs records added in the previous frame >> record metadata patch, and recovers return addresses replaced by >> function graph tracing and kretprobes. > >> This commit introduces arch_stack_walk_reliable() but does not yet >> select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig >> patch so this commit can be reviewed and bisected as a pure unwinder >> replacement. Until that Kconfig change lands, livepatch is not yet >> enabled and arch_stack_walk_reliable() has no in-tree caller. > >> Three related callers are updated to keep the same frame-record >> assumptions everywhere: > >> * Function graph tracing: the old RISC-V unwinder matched function >> graph return-stack entries by the saved return-address slot. That >> was consistent with the static mcount path, but not with the dynamic >> ftrace path where the parent slot is ftrace_regs::ra. Use the >> architectural frame pointer as the function graph return-address >> cookie, matching the kunwind walker. > >> * Perf callchains: route kernel callchain collection through >> arch_stack_walk() so perf sees the same frame-pointer unwind >> behaviour as dump_stack() and the upcoming livepatch path. > >> * dump_backtrace() / __get_wchan() / show_stack(): these now go >> through arch_stack_walk(); the explicit "Call Trace:" header is >> moved into dump_backtrace() to preserve the original output. > >> The non-frame-pointer fallback walker is kept untouched for >> !CONFIG_FRAME_POINTER builds. > >> Assisted-by: Qoder:Qwen-3.8-MAX-Preview >> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> >> --- >> arch/riscv/kernel/ftrace.c | 6 +- >> arch/riscv/kernel/perf_callchain.c | 2 +- >> arch/riscv/kernel/stacktrace.c | 561 ++++++++++++++++++++++++----- >> 3 files changed, 473 insertions(+), 96 deletions(-) > >> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c >> index be8b685144174..0facd824c911d 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; >> } > > > Are these two changes fixing a bug in the current code (that is independent > from the rest of this patch)? > > Without your changes, function_graph_enter_regs writes a struct ftrace_ret_stack > with retp == the saved ra. > > And the only place where retp is fetched and checked is > > function_get_true_parent_ip > ftrace_graph_ret_addr(..., (uintptr_t *)ftrace_regs_get_stack_pointer(fregs)); > > The stored retp is checked against the stack pointer. > > Your changes store fp instead of ra, this seems the right thing to do. (And > it's in line with other archs.) > > Should those two changes be in a separate patch (that can be backported to > stable kernels)? Hi, Martin, Thanks for the sharp question. You are right that the stored cookie is broken. What ftrace_graph_func() passes as retp is &arch_ftrace_regs(fregs)->ra, i.e. the slot inside the transient ftrace trampoline frame, which is popped as soon as the callback returns. That has been the case since 35e61e8827ee8 ("riscv: ftrace: Make function graph use ftrace directly") introduced ftrace_graph_func() in v6.8. (The code was copied from arm64's ftrace_graph_func(), but arm64 passes (void *)frame_pointer there - see 41705c4262aac for the current shape.) 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. I will split these two changes into a separate patch and cc stable. Thanks. Shuai ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (4 preceding siblings ...) 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:38 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue ` (2 subsequent siblings) 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Now that the metadata frame records, the kunwind state machine and arch_stack_walk_reliable() are all in place, advertise the capability to the rest of the kernel: * select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so only the configurations with the tested metadata records and FP-based reliable walker enable it. * select HAVE_LIVEPATCH under the same condition and source kernel/livepatch/Kconfig so the livepatch menu is reachable from the RISC-V configuration. The 64BIT dependency is conservative scoping rather than a hard technical requirement: the metadata frame record, kunwind state machine and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack frame-record adjustment fixes a latent RV32 issue. However, the syscall livepatch selftest and module relocation path have only been exercised on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up once RV32 has equivalent coverage. This is split out from the unwinder change so the policy decision and the implementation can be reviewed and reverted independently. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/Kconfig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index bf2e81d258c4d..b92e02481198f 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -186,6 +186,7 @@ config RISCV select HAVE_KRETPROBES # https://github.com/ClangBuiltLinux/linux/issues/1881 select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD + select HAVE_LIVEPATCH if FRAME_POINTER && 64BIT select HAVE_MOVE_PMD select HAVE_MOVE_PUD select HAVE_PAGE_SIZE_4KB @@ -196,6 +197,7 @@ config RISCV select HAVE_POSIX_CPU_TIMERS_TASK_WORK select HAVE_PREEMPT_DYNAMIC_KEY select HAVE_REGS_AND_STACK_ACCESS_API + select HAVE_RELIABLE_STACKTRACE if FRAME_POINTER && 64BIT select HAVE_RETHOOK select HAVE_RSEQ select HAVE_RUST if RUSTC_SUPPORTS_RISCV && CC_IS_CLANG @@ -1411,3 +1413,5 @@ endmenu # "CPU Power Management" source "arch/riscv/kvm/Kconfig" source "drivers/acpi/Kconfig" + +source "kernel/livepatch/Kconfig" -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH 2026-09-14 9:26 ` [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue @ 2026-09-14 9:38 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:38 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > Now that the metadata frame records, the kunwind state machine and > arch_stack_walk_reliable() are all in place, advertise the capability > to the rest of the kernel: > > * select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so > only the configurations with the tested metadata records and > FP-based reliable walker enable it. > * select HAVE_LIVEPATCH under the same condition and source > kernel/livepatch/Kconfig so the livepatch menu is reachable from > the RISC-V configuration. > > The 64BIT dependency is conservative scoping rather than a hard > technical requirement: the metadata frame record, kunwind state machine > and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack > frame-record adjustment fixes a latent RV32 issue. However, the syscall > livepatch selftest and module relocation path have only been exercised > on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up > once RV32 has equivalent coverage. > > This is split out from the unwinder change so the policy decision and > the implementation can be reviewed and reverted independently. > > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=6 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (5 preceding siblings ...) 2026-09-14 9:26 ` [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue @ 2026-09-14 9:26 ` Shuai Xue 2026-09-14 9:38 ` sashiko-bot 2026-09-29 7:28 ` [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Martin Kaiser 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue 8 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-09-14 9:26 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> The syscall livepatch selftest resolves and patches a syscall wrapper symbol. To use that test for RISC-V livepatch validation, add the RISC-V FN_PREFIX definition for ARCH_HAS_SYSCALL_WRAPPER. Without this macro, the syscall livepatch selftest cannot resolve the RISC-V target symbol, and the syscall-related livepatch test fails on RISC-V. Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Assisted-by: Qoder:Qwen-3.8-MAX-Preview Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- .../testing/selftests/livepatch/test_modules/test_klp_syscall.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c index 08aacc0e14de9..9baa2a5f84c95 100644 --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c @@ -24,6 +24,8 @@ #define FN_PREFIX __s390x_ #elif defined(__aarch64__) #define FN_PREFIX __arm64_ + #elif defined(__riscv) + #define FN_PREFIX __riscv_ #elif defined(__powerpc__) #define FN_PREFIX #else -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix 2026-09-14 9:26 ` [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue @ 2026-09-14 9:38 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-09-14 9:38 UTC (permalink / raw) To: Shuai Xue; +Cc: linux-trace-kernel, linux-perf-users > From: Wang Han <wanghan@linux.alibaba.com> > > The syscall livepatch selftest resolves and patches a syscall wrapper > symbol. To use that test for RISC-V livepatch validation, add the > RISC-V FN_PREFIX definition for ARCH_HAS_SYSCALL_WRAPPER. > > Without this macro, the syscall livepatch selftest cannot resolve the > RISC-V target symbol, and the syscall-related livepatch test fails on > RISC-V. > > Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com> > Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> > Acked-by: Miroslav Benes <mbenes@suse.cz> > Assisted-by: Qoder:Qwen-3.8-MAX-Preview > Signed-off-by: Wang Han <wanghan@linux.alibaba.com> > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914092648.51254-1-xueshuai@linux.alibaba.com?part=7 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (6 preceding siblings ...) 2026-09-14 9:26 ` [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue @ 2026-09-29 7:28 ` Martin Kaiser 2026-10-08 11:42 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue 8 siblings, 1 reply; 39+ messages in thread From: Martin Kaiser @ 2026-09-29 7:28 UTC (permalink / raw) To: Shuai Xue Cc: palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest Thus wrote Shuai Xue (xueshuai@linux.alibaba.com): > Hi, > This is v6 of the RISC-V reliable stack unwinding series for livepatch. > Wang Han has left our team, so I am taking over this series from now on. > The series adds explicit frame-record metadata, preserves the frame > pointer through dynamic ftrace, and replaces the open-coded stack walker > with a conservative frame-pointer based unwinder. It then enables the > RISC-V livepatch capability and adds the syscall-wrapper selftest prefix. > Changes since v5: > * Rebase onto the current riscv/for-next; no functional changes. > * Fold in the tags collected from the v5 thread: > - Reviewed-by: Shuai Xue on 2/7, 3/7, 4/7 and 6/7 > - Acked-by: Miroslav Benes on 7/7 > * Add my sign-off as the new sender. > Since the patches are otherwise identical to v5, the v5 validation > results still apply. For reference, from the v5 cover letter: > * Built Linux 7.2.0-rc6 with riscv64-unknown-linux-gnu-gcc 15.2.0 > and binutils 2.46. > * Standard livepatch configuration built successfully, including > Image, vmlinux, modules, compile_commands.json and GDB scripts. > * CONFIG_IRQ_STACKS=n variant linked successfully with GCC. > * QEMU 8.2.2, 1 CPU: livepatch selftests PASS: 7, SKIP: 1, > FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported. > * QEMU 8.2.2, 4 CPUs: livepatch selftests PASS: 7, SKIP: 1, > FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported. > * No BUG, WARNING, Oops, panic, unreliable stack or Call Trace found > by the validation log scans. Expected negative livepatch errors are > part of the selftests. > v5 was posted in early August and has been quiet since then. I would > appreciate it if the series could be considered for the upcoming merge > window. Happy to address any remaining feedback. > Base: > riscv/for-next 77ae27fd98f3 ("Merge tag 'printk-for-7.3' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux") > Previous versions: > v5: https://lore.kernel.org/r/20260804103626.2803112-1-wanghan@linux.alibaba.com > v4 RESEND: https://lore.kernel.org/r/20260629072713.3273743-1-wanghan@linux.alibaba.com > v3: https://lore.kernel.org/r/cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com > v2: https://lore.kernel.org/r/20260528082310.1994388-1-wanghan@linux.alibaba.com > v1: https://lore.kernel.org/r/20260527123530.2593918-1-wanghan@linux.alibaba.com > Assisted-by: Qoder:Qwen-3.8-MAX-Preview Thanks for these patches. I tried them on a standard rv64 qemu and on a rv64 cheri system (cheri-linux.org). The latter needed some minor tweaks, but in both cases, I could get livepatch to work. Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch 2026-09-29 7:28 ` [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Martin Kaiser @ 2026-10-08 11:42 ` Shuai Xue 2026-10-12 8:44 ` Martin Kaiser 0 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-10-08 11:42 UTC (permalink / raw) To: Martin Kaiser, Palmer Dabbelt, Paul Walmsley Cc: pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest On 9/29/26 3:28 PM, Martin Kaiser wrote: > Thus wrote Shuai Xue (xueshuai@linux.alibaba.com): > >> Hi, > >> This is v6 of the RISC-V reliable stack unwinding series for livepatch. >> Wang Han has left our team, so I am taking over this series from now on. > >> The series adds explicit frame-record metadata, preserves the frame >> pointer through dynamic ftrace, and replaces the open-coded stack walker >> with a conservative frame-pointer based unwinder. It then enables the >> RISC-V livepatch capability and adds the syscall-wrapper selftest prefix. > >> Changes since v5: >> * Rebase onto the current riscv/for-next; no functional changes. >> * Fold in the tags collected from the v5 thread: >> - Reviewed-by: Shuai Xue on 2/7, 3/7, 4/7 and 6/7 >> - Acked-by: Miroslav Benes on 7/7 >> * Add my sign-off as the new sender. > >> Since the patches are otherwise identical to v5, the v5 validation >> results still apply. For reference, from the v5 cover letter: >> * Built Linux 7.2.0-rc6 with riscv64-unknown-linux-gnu-gcc 15.2.0 >> and binutils 2.46. >> * Standard livepatch configuration built successfully, including >> Image, vmlinux, modules, compile_commands.json and GDB scripts. >> * CONFIG_IRQ_STACKS=n variant linked successfully with GCC. >> * QEMU 8.2.2, 1 CPU: livepatch selftests PASS: 7, SKIP: 1, >> FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported. >> * QEMU 8.2.2, 4 CPUs: livepatch selftests PASS: 7, SKIP: 1, >> FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported. >> * No BUG, WARNING, Oops, panic, unreliable stack or Call Trace found >> by the validation log scans. Expected negative livepatch errors are >> part of the selftests. > >> v5 was posted in early August and has been quiet since then. I would >> appreciate it if the series could be considered for the upcoming merge >> window. Happy to address any remaining feedback. > >> Base: >> riscv/for-next 77ae27fd98f3 ("Merge tag 'printk-for-7.3' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux") > >> Previous versions: >> v5: https://lore.kernel.org/r/20260804103626.2803112-1-wanghan@linux.alibaba.com >> v4 RESEND: https://lore.kernel.org/r/20260629072713.3273743-1-wanghan@linux.alibaba.com >> v3: https://lore.kernel.org/r/cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com >> v2: https://lore.kernel.org/r/20260528082310.1994388-1-wanghan@linux.alibaba.com >> v1: https://lore.kernel.org/r/20260527123530.2593918-1-wanghan@linux.alibaba.com > >> Assisted-by: Qoder:Qwen-3.8-MAX-Preview > > Thanks for these patches. > > I tried them on a standard rv64 qemu and on a rv64 cheri system > (cheri-linux.org). The latter needed some minor tweaks, but in > both cases, I could get livepatch to work. > > Reviewed-by: Martin Kaiser <martin@kaiser.cx> > Tested-by: Martin Kaiser <martin@kaiser.cx> Hi Martin, Thank you very much for testing the series and for the tags! It is great to hear that livepatch works both on a standard rv64 QEMU and on the rv64 CHERI system. Could you share what the minor tweaks for CHERI were? If any of them are generic -- for example frame-record size/alignment or pointer-width assumptions in the unwinder -- it may be worth folding them into this series, so the cheri-linux tree has less to carry downstream. If they are purely CHERI-specific, a pointer to your tree would still be a useful reference. +Palmer, Paul, gentle ping on this series. It has been under review since late May (v1 on May 27, now in its sixth revision) and the tags collected so far are: * Acked-by: Miroslav Benes <mbenes@suse.cz> (7/7, livepatch) * Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com> (7/7) * Reviewed-by: Martin Kaiser <martin@kaiser.cx> (whole series) * Tested-by: Martin Kaiser <martin@kaiser.cx> (whole series, rv64 QEMU and rv64 CHERI, livepatch working on both) * Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> (2/7, 3/7, 4/7, 6/7) v6 still applies cleanly onto the current riscv/for-next (52318cf0fa6e). If it helps, I can send a v7 that folds in Martin's Reviewed-by/Tested-by tags. Could the series be considered for the upcoming 7.4 merge window? Thanks, Shuai Xue ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch 2026-10-08 11:42 ` Shuai Xue @ 2026-10-12 8:44 ` Martin Kaiser 0 siblings, 0 replies; 39+ messages in thread From: Martin Kaiser @ 2026-10-12 8:44 UTC (permalink / raw) To: Shuai Xue Cc: Palmer Dabbelt, Paul Walmsley, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest Hi Shuai, Thus wrote Shuai Xue (xueshuai@linux.alibaba.com): > It is great to hear that livepatch works both on a standard rv64 QEMU > and on the rv64 CHERI system. Could you share what the minor tweaks > for CHERI were? If any of them are generic -- for example frame-record > size/alignment or pointer-width assumptions in the unwinder -- it may > be worth folding them into this series, so the cheri-linux tree has > less to carry downstream. If they are purely CHERI-specific, a pointer > to your tree would still be a useful reference. the cheri changes are simple fixups of data types and assembly instructions. This isn't something that can be upstreamed at this point. Eventually, it'll end up in the cheri linux tree at https://github.com/CHERI-Alliance/linux but there's more cleanup to do first. I just wanted to show my interest in getting your patches merged. Thanks, Martin > +Palmer, Paul, gentle ping on this series. > It has been under review since late May (v1 on May 27, now in its sixth > revision) and the tags collected so far are: > * Acked-by: Miroslav Benes <mbenes@suse.cz> (7/7, livepatch) > * Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com> (7/7) > * Reviewed-by: Martin Kaiser <martin@kaiser.cx> (whole series) > * Tested-by: Martin Kaiser <martin@kaiser.cx> (whole series, > rv64 QEMU and rv64 CHERI, livepatch working on both) > * Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> (2/7, 3/7, 4/7, 6/7) > v6 still applies cleanly onto the current riscv/for-next > (52318cf0fa6e). If it helps, I can send a v7 that folds in Martin's > Reviewed-by/Tested-by tags. > Could the series be considered for the upcoming 7.4 merge window? > Thanks, > Shuai Xue ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 00/10] riscv: Add reliable stack unwinding for livepatch 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue ` (7 preceding siblings ...) 2026-09-29 7:28 ` [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Martin Kaiser @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() Shuai Xue ` (10 more replies) 8 siblings, 11 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest Hi, This is v7 of the RISC-V reliable stack unwinding series for livepatch. The series starts with three standalone fixes for the existing stack walkers: Karl Mehltretter's fix for arch_stack_walk() reporting the walker's own frame, its KUnit test, and a fix for the function-graph return-address cookie that Martin Kaiser asked to be split out. The main part of the series then adds explicit frame-record metadata, preserves the frame pointer through dynamic ftrace, replaces the open-coded stack walker with a conservative frame-pointer based unwinder, enables the RISC-V livepatch capability and adds the syscall-wrapper selftest prefix. Changes since v6: * Rebase onto the current riscv/for-next (based on 7.4-rc1). * Fold in Karl Mehltretter's two patches [1] as 1/10 and 2/10. Patch 1/10 stops arch_stack_walk() from reporting the walker's own frame as the first entry, which corrupts the skip-frame accounting of stack_trace_save() users (page_owner, perf), and adjusts return_address() level accounting accordingly. Patch 2/10 adds a KUnit test for the start frames. Both patches keep their original authorship; I added my Reviewed-by/Tested-by and sign-off. I asked Karl on 3 November whether he would like to see his patches folded into this series; he has not replied so far. Since 8/10 reuses the walker extraction from 1/10 for its !CONFIG_FRAME_POINTER fallback, his patches are included here to keep the series self-contained. * Split the function-graph return-address cookie change out of the old 5/7, as suggested by Martin Kaiser. It is now a separate patch (3/10) that also fixes the old walker's cookie key, so it stands on its own without the rest of the series and can be picked independently; it is marked Fixes: and Cc: stable. * Reworked the !CONFIG_FRAME_POINTER fallback of the unwinder switch patch (8/10) to use the __always_inline __walk_stackframe() helper from 1/10, so both walkers report the same start frame. * Collected tags from the v6 review thread: Reviewed-by and Tested-by: Martin Kaiser for the patches carried over from v6 (rv64 QEMU and rv64 CHERI builds, livepatch working on both). The patches carried over from v6 are unchanged, except for the reworked !CONFIG_FRAME_POINTER fallback in 8/10, so the v6 validation results still apply to them. The v7-specific changes were covered by the fresh build and boot runs described below: * Built with riscv64-unknown-linux-gnu-gcc 15.2.0 and binutils 2.46, including a CONFIG_IRQ_STACKS=n variant. * QEMU 8.2.2, 1 and 4 CPUs: livepatch selftests PASS: 7, SKIP: 1, FAIL: 0; ftrace function graph 3 passed, 0 failed, 3 unsupported. * No BUG, WARNING, Oops, panic, unreliable stack or Call Trace found by the validation log scans. Patches 1-2/10 were tested by Karl with the KUnit suite from 2/10 on QEMU's virt machine with clang 22 (LLVM=1) and GCC 15.2, and on a BeagleV Ahead (TH1520), where all five test cases pass. In addition, the combined v7 tree was freshly built and booted: * Built with riscv64-unknown-linux-gnu-gcc 16.2.0 on top of 7.4-rc1: a full livepatch configuration (CONFIG_LIVEPATCH=y with dynamic ftrace and function graph tracer) and a !CONFIG_FRAME_POINTER variant both link successfully. * QEMU 11.1.1, rv64 virt, 4 CPUs: the KUnit suite from 2/10 passes 5/5 on the frame-pointer build; the !CONFIG_FRAME_POINTER build boots and its panic backtrace (no initramfs, by design) unwinds cleanly. Base: riscv/for-next 6628e7d4c6b2 ("Linux 7.4-rc1") Previous versions: v6: https://lore.kernel.org/r/20260914092648.51254-1-xueshuai@linux.alibaba.com v5: https://lore.kernel.org/r/20260804103626.2803112-1-wanghan@linux.alibaba.com v4 RESEND: https://lore.kernel.org/r/20260629072713.3273743-1-wanghan@linux.alibaba.com v3: https://lore.kernel.org/r/cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com v2: https://lore.kernel.org/r/20260528082310.1994388-1-wanghan@linux.alibaba.com v1: https://lore.kernel.org/r/20260527123530.2593918-1-wanghan@linux.alibaba.com [1] https://lore.kernel.org/r/20261010132712.46931-1-kmehltretter@gmail.com Karl Mehltretter (2): riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() riscv: Add KUnit test for stack trace start frames Shuai Xue (1): riscv: ftrace: use the frame pointer as the function graph return-address cookie Wang Han (7): riscv: stacktrace: Add frame record metadata riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o riscv: ftrace: always preserve s0 in dynamic ftrace register frame riscv: stacktrace: introduce stack-bound tracking helpers riscv: stacktrace: switch to frame-pointer based unwinder riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH selftests/livepatch: Add RISC-V syscall wrapper prefix arch/riscv/Kconfig | 4 + arch/riscv/include/asm/ptrace.h | 9 + arch/riscv/include/asm/stacktrace.h | 67 ++- arch/riscv/include/asm/stacktrace/common.h | 159 +++++ arch/riscv/include/asm/stacktrace/frame.h | 53 ++ arch/riscv/kernel/Makefile | 6 + arch/riscv/kernel/asm-offsets.c | 6 + arch/riscv/kernel/entry.S | 39 +- arch/riscv/kernel/ftrace.c | 6 +- arch/riscv/kernel/head.S | 23 + arch/riscv/kernel/mcount-dyn.S | 4 - arch/riscv/kernel/perf_callchain.c | 2 +- arch/riscv/kernel/process.c | 33 +- arch/riscv/kernel/return_address.c | 2 +- arch/riscv/kernel/stacktrace.c | 583 +++++++++++++++--- arch/riscv/kernel/tests/Kconfig.debug | 12 + arch/riscv/kernel/tests/Makefile | 1 + arch/riscv/kernel/tests/stacktrace_test.c | 142 +++++ .../livepatch/test_modules/test_klp_syscall.c | 2 + 19 files changed, 1043 insertions(+), 110 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/common.h create mode 100644 arch/riscv/include/asm/stacktrace/frame.h create mode 100644 arch/riscv/kernel/tests/stacktrace_test.c base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4 -- 2.39.3 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:52 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames Shuai Xue ` (9 subsequent siblings) 10 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Karl Mehltretter <kmehltretter@gmail.com> stack_trace_save() uses skipnr + 1 because arch_stack_walk() is expected to first report the return into stack_trace_save(). With FRAME_POINTER, the RISC-V arch_stack_walk() calls walk_stackframe(), which starts from its own frame and first reports the return into arch_stack_walk(). stack_trace_save() therefore starts one frame too early. For example, page_owner records begin at __set_page_owner() instead of the allocation site. Move the walker into an always-inlined helper and call it directly from arch_stack_walk(). Keep walk_stackframe() as a wrapper for its existing callers. return_address() skips level + 3 entries, while the arm64 implementation skips level + 2. After removing the extra frame, use level + 2 so ftrace_return_address() and CALLER_ADDR results do not shift by one caller. Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)") Link: https://lore.kernel.org/r/20261010132712.46931-1-kmehltretter@gmail.com Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Tested-by: Shuai Xue <xueshuai@linux.alibaba.com> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/return_address.c | 2 +- arch/riscv/kernel/stacktrace.c | 40 ++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/arch/riscv/kernel/return_address.c b/arch/riscv/kernel/return_address.c index c8115ec8fb30..4da1cfd249d3 100644 --- a/arch/riscv/kernel/return_address.c +++ b/arch/riscv/kernel/return_address.c @@ -33,7 +33,7 @@ noinline void *return_address(unsigned int level) { struct return_address_data data; - data.level = level + 3; + data.level = level + 2; data.addr = NULL; arch_stack_walk(save_return_addr, &data, current, NULL); diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c index c7555447149b..28c5744889ba 100644 --- a/arch/riscv/kernel/stacktrace.c +++ b/arch/riscv/kernel/stacktrace.c @@ -45,8 +45,16 @@ static inline int fp_is_valid(unsigned long fp, unsigned long sp) return !(fp < low || fp > high || fp & 0x07); } -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg) +/* + * Always inlined so that the frame the walk starts from is the caller's own, + * and the first entry reported for the current task is the caller's return + * address. arch_stack_walk() relies on that to report the same frames as + * the other architectures, see the comment there. + */ +static __always_inline void __walk_stackframe(struct task_struct *task, + struct pt_regs *regs, + bool (*fn)(void *, unsigned long), + void *arg) { unsigned long fp, sp, pc; int graph_idx = 0; @@ -59,6 +67,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, } else if (task == NULL || task == current) { fp = (unsigned long)__builtin_frame_address(0); sp = current_stack_pointer; + /* Placeholder for the frame the walk starts from, never reported. */ pc = (unsigned long)walk_stackframe; level = -1; } else { @@ -102,10 +111,18 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, } } +void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) +{ + __walk_stackframe(task, regs, fn, arg); +} + #else /* !CONFIG_FRAME_POINTER */ -void notrace walk_stackframe(struct task_struct *task, - struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) +static __always_inline void __walk_stackframe(struct task_struct *task, + struct pt_regs *regs, + bool (*fn)(void *, unsigned long), + void *arg) { unsigned long sp, pc; unsigned long *ksp; @@ -133,6 +150,12 @@ void notrace walk_stackframe(struct task_struct *task, } } +void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) +{ + __walk_stackframe(task, regs, fn, arg); +} + #endif /* CONFIG_FRAME_POINTER */ static bool print_trace_address(void *arg, unsigned long pc) @@ -176,10 +199,17 @@ unsigned long __get_wchan(struct task_struct *task) return pc; } +/* + * The generic code expects the first entry for the current task to be the + * return address of arch_stack_walk() itself, the way x86 and arm64 report + * it, and stack_trace_save() sizes its skip count accordingly. Walking from + * inside a called walk_stackframe() adds that function's own frame on top, + * so inline the walker here and start from this frame instead. + */ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, struct task_struct *task, struct pt_regs *regs) { - walk_stackframe(task, regs, consume_entry, cookie); + __walk_stackframe(task, regs, consume_entry, cookie); } /* -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() 2026-11-10 1:37 ` [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() Shuai Xue @ 2026-11-10 1:52 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-11-10 1:52 UTC (permalink / raw) To: Shuai Xue, palmer, pjw, aou Cc: alex, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, jkchen, linux-trace-kernel, linux-perf-users, live-patching Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] riscv: stacktrace: off-by-one frame unwind in __walk_stackframe() for !FRAME_POINTER -- commit 0ad8b77869cf04ae7e9c217cd0f43219461f68f7 Author: Karl Mehltretter <kmehltretter@gmail.com> riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() This commit moves the stack walker into an always-inlined __walk_stackframe() helper called directly from arch_stack_walk() and reduces the skip count in return_address() from level + 3 to level + 2. Link: https://lore.kernel.org/r/20261010132712.46931-1-kmehltretter@gmail.com > diff --git a/arch/riscv/kernel/return_address.c b/arch/riscv/kernel/return_address.c > index c8115ec8fb304..4da1cfd249d3d 100644 > --- a/arch/riscv/kernel/return_address.c > +++ b/arch/riscv/kernel/return_address.c > @@ -33,7 +33,7 @@ noinline void *return_address(unsigned int level) > { > struct return_address_data data; > > - data.level = level + 3; > + data.level = level + 2; > data.addr = NULL; > > arch_stack_walk(save_return_addr, &data, current, NULL); > diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c > index c7555447149bf..28c5744889bac 100644 > --- a/arch/riscv/kernel/stacktrace.c > +++ b/arch/riscv/kernel/stacktrace.c [ ... ] > @@ -102,10 +111,18 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > } > } > > +void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > + bool (*fn)(void *, unsigned long), void *arg) > +{ > + __walk_stackframe(task, regs, fn, arg); > +} > + > #else /* !CONFIG_FRAME_POINTER */ > > -void notrace walk_stackframe(struct task_struct *task, > - struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) > +static __always_inline void __walk_stackframe(struct task_struct *task, > + struct pt_regs *regs, > + bool (*fn)(void *, unsigned long), > + void *arg) > { > unsigned long sp, pc; > unsigned long *ksp; [Severity: Medium] Does the !CONFIG_FRAME_POINTER version of __walk_stackframe() still report walk_stackframe as its first entry before scanning the stack? arch/riscv/kernel/stacktrace.c:__walk_stackframe() { ... } else if (task == NULL || task == current) { sp = current_stack_pointer; pc = (unsigned long)walk_stackframe; } else { ... while (!kstack_end(ksp)) { if (__kernel_text_address(pc) && unlikely(!fn(arg, pc))) break; pc = READ_ONCE_NOCHECK(*ksp++); } } Unlike the CONFIG_FRAME_POINTER version, which initializes level = -1 so the initial walk_stackframe placeholder is skipped, the !CONFIG_FRAME_POINTER loop passes pc = (unsigned long)walk_stackframe directly to fn() on the first iteration before reading arch_stack_walk()'s saved return address from *ksp++. As a result, when CONFIG_FRAME_POINTER is disabled, stack_trace_save() still starts one frame too early, and reducing data.level from level + 3 to level + 2 in return_address() causes return_address(), ftrace_return_address(), and CALLER_ADDRx to stop one caller too early. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110013750.36800-1-xueshuai@linux.alibaba.com?part=1 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue 2026-11-10 1:37 ` [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:50 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 03/10] riscv: ftrace: use the frame pointer as the function graph return-address cookie Shuai Xue ` (8 subsequent siblings) 10 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Karl Mehltretter <kmehltretter@gmail.com> stack_trace_save() and return_address() depend on the first frame reported by arch_stack_walk(), but no test checks their frame numbering. Add a test that calls both through a chain of three noinline functions and compares the symbol of the result with the function expected for each skip count or level. Require FRAME_POINTER because the stack-scanning walker cannot provide an exact start frame. Link: https://lore.kernel.org/r/20261010132712.46931-2-kmehltretter@gmail.com Tested-by: Shuai Xue <xueshuai@linux.alibaba.com> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/tests/Kconfig.debug | 12 ++ arch/riscv/kernel/tests/Makefile | 1 + arch/riscv/kernel/tests/stacktrace_test.c | 142 ++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 arch/riscv/kernel/tests/stacktrace_test.c diff --git a/arch/riscv/kernel/tests/Kconfig.debug b/arch/riscv/kernel/tests/Kconfig.debug index 40f8dafffa0a..eb79df5b2c5a 100644 --- a/arch/riscv/kernel/tests/Kconfig.debug +++ b/arch/riscv/kernel/tests/Kconfig.debug @@ -42,6 +42,18 @@ config RISCV_KPROBES_KUNIT If unsure, say N. +config RISCV_STACKTRACE_KUNIT_TEST + bool "KUnit tests for RISC-V stack trace start frames" + depends on KUNIT && STACKTRACE && FRAME_POINTER && KALLSYMS + help + Check that stack_trace_save() starts at its caller and skips exactly + the requested number of frames through a known call chain, and that + ftrace_return_address(), which CALLER_ADDR1 and up are built on, + returns the caller it was asked for. This catches a stack walker + that exposes its own implementation frames to its users. + + If unsure, say N. + endif # RUNTIME_TESTING_MENU endmenu # "arch/riscv/kernel runtime Testing" diff --git a/arch/riscv/kernel/tests/Makefile b/arch/riscv/kernel/tests/Makefile index 407e7e6c28dc..9ca4135bffb5 100644 --- a/arch/riscv/kernel/tests/Makefile +++ b/arch/riscv/kernel/tests/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_RISCV_MODULE_LINKING_KUNIT) += module_test/ obj-$(CONFIG_RISCV_KPROBES_KUNIT) += kprobes/ +obj-$(CONFIG_RISCV_STACKTRACE_KUNIT_TEST) += stacktrace_test.o diff --git a/arch/riscv/kernel/tests/stacktrace_test.c b/arch/riscv/kernel/tests/stacktrace_test.c new file mode 100644 index 000000000000..bb299a106773 --- /dev/null +++ b/arch/riscv/kernel/tests/stacktrace_test.c @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Check where stack_trace_save() and ftrace_return_address() start. */ +#include <kunit/test.h> +#include <linux/ftrace.h> +#include <linux/kallsyms.h> +#include <linux/module.h> +#include <linux/stacktrace.h> + +#define RISCV_STACKTRACE_DEPTH 16 + +static noinline unsigned int +riscv_stacktrace_leaf(unsigned long *entries, unsigned int skip) +{ + unsigned int count = stack_trace_save(entries, RISCV_STACKTRACE_DEPTH, skip); + + barrier(); + return count; +} + +static noinline unsigned int +riscv_stacktrace_middle(unsigned long *entries, unsigned int skip) +{ + unsigned int count = riscv_stacktrace_leaf(entries, skip); + + barrier(); + return count; +} + +static noinline unsigned int +riscv_stacktrace_outer(unsigned long *entries, unsigned int skip) +{ + unsigned int count = riscv_stacktrace_middle(entries, skip); + + barrier(); + return count; +} + +static void riscv_stacktrace_check_skip(struct kunit *test, unsigned int skip) +{ + static const char * const expected[] = { + "riscv_stacktrace_leaf", + "riscv_stacktrace_middle", + "riscv_stacktrace_outer", + }; + unsigned long entries[RISCV_STACKTRACE_DEPTH]; + char symbol[KSYM_SYMBOL_LEN]; + unsigned int count; + + count = riscv_stacktrace_outer(entries, skip); + KUNIT_ASSERT_GT(test, count, 0U); + sprint_symbol_no_offset(symbol, entries[0]); + kunit_info(test, "skip=%u first=%s count=%u\n", skip, symbol, count); + KUNIT_EXPECT_STREQ(test, symbol, expected[skip]); +} + +static void riscv_stacktrace_skip_zero(struct kunit *test) +{ + riscv_stacktrace_check_skip(test, 0); +} + +static void riscv_stacktrace_skip_one(struct kunit *test) +{ + riscv_stacktrace_check_skip(test, 1); +} + +static void riscv_stacktrace_skip_two(struct kunit *test) +{ + riscv_stacktrace_check_skip(test, 2); +} + +static noinline void *riscv_return_address_leaf(unsigned int level) +{ + void *addr = (void *)ftrace_return_address(level); + + barrier(); + return addr; +} + +static noinline void *riscv_return_address_middle(unsigned int level) +{ + void *addr = riscv_return_address_leaf(level); + + barrier(); + return addr; +} + +static noinline void *riscv_return_address_outer(unsigned int level) +{ + void *addr = riscv_return_address_middle(level); + + barrier(); + return addr; +} + +/* + * ftrace_return_address(n) is what CALLER_ADDRn expands to. Like + * __builtin_return_address(n), level 0 is the caller of the function that + * uses it. + */ +static void riscv_return_address_check(struct kunit *test, unsigned int level) +{ + static const char * const expected[] = { + "riscv_return_address_middle", + "riscv_return_address_outer", + }; + char symbol[KSYM_SYMBOL_LEN]; + void *addr; + + addr = riscv_return_address_outer(level); + KUNIT_ASSERT_NOT_NULL(test, addr); + sprint_symbol_no_offset(symbol, (unsigned long)addr); + kunit_info(test, "level=%u caller=%s\n", level, symbol); + KUNIT_EXPECT_STREQ(test, symbol, expected[level]); +} + +static void riscv_return_address_level_zero(struct kunit *test) +{ + riscv_return_address_check(test, 0); +} + +static void riscv_return_address_level_one(struct kunit *test) +{ + riscv_return_address_check(test, 1); +} + +static struct kunit_case riscv_stacktrace_cases[] = { + KUNIT_CASE(riscv_stacktrace_skip_zero), + KUNIT_CASE(riscv_stacktrace_skip_one), + KUNIT_CASE(riscv_stacktrace_skip_two), + KUNIT_CASE(riscv_return_address_level_zero), + KUNIT_CASE(riscv_return_address_level_one), + {} +}; + +static struct kunit_suite riscv_stacktrace_suite = { + .name = "riscv-stacktrace-skip", + .test_cases = riscv_stacktrace_cases, +}; + +kunit_test_suite(riscv_stacktrace_suite); + +MODULE_LICENSE("GPL"); -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames 2026-11-10 1:37 ` [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames Shuai Xue @ 2026-11-10 1:50 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-11-10 1:50 UTC (permalink / raw) To: Shuai Xue, palmer, pjw, aou Cc: alex, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, jkchen, linux-trace-kernel, linux-perf-users, live-patching Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] riscv: vmlinux link failure when CONFIG_KUNIT=m with RISCV_STACKTRACE_KUNIT_TEST -- commit 88e1ac2e959d970cabee9d276d02925cc3d44f40 Author: Karl Mehltretter <kmehltretter@gmail.com> riscv: Add KUnit test for stack trace start frames This commit adds a KUnit test suite to verify the starting frame and skip counts of stack_trace_save() and ftrace_return_address() on RISC-V. Link: https://lore.kernel.org/r/20261010132712.46931-2-kmehltretter@gmail.com > diff --git a/arch/riscv/kernel/tests/Kconfig.debug b/arch/riscv/kernel/tests/Kconfig.debug > index 40f8dafffa0ab..eb79df5b2c5a4 100644 > --- a/arch/riscv/kernel/tests/Kconfig.debug > +++ b/arch/riscv/kernel/tests/Kconfig.debug > @@ -42,6 +42,18 @@ config RISCV_KPROBES_KUNIT > > If unsure, say N. > > +config RISCV_STACKTRACE_KUNIT_TEST > + bool "KUnit tests for RISC-V stack trace start frames" > + depends on KUNIT && STACKTRACE && FRAME_POINTER && KALLSYMS [Severity: Medium] Does declaring RISCV_STACKTRACE_KUNIT_TEST as bool while depending on tristate KUNIT break the vmlinux link when CONFIG_KUNIT=m and CONFIG_RISCV_STACKTRACE_KUNIT_TEST=y (such as under allmodconfig)? In that configuration, stacktrace_test.o is built into vmlinux while the KUnit core symbols it calls (__kunit_do_failed_assertion, kunit_log_append, etc.) live in kunit.ko, similar to the issue fixed for RISCV_KPROBES_KUNIT in 5fada1605733 ("riscv: tests: Make RISCV_KPROBES_KUNIT tristate"). Should this depend on KUNIT=y instead (since building stacktrace_test as a module would also cause sprint_symbol_no_offset() in riscv_stacktrace_check_skip() and riscv_return_address_check() to append " [stacktrace_test]" to the symbol names)? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110013750.36800-1-xueshuai@linux.alibaba.com?part=2 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 03/10] riscv: ftrace: use the frame pointer as the function graph return-address cookie 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue 2026-11-10 1:37 ` [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() Shuai Xue 2026-11-10 1:37 ` [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata Shuai Xue ` (7 subsequent siblings) 10 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest 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@kaiser.cx> Cc: stable@vger.kernel.org Signed-off-by: Shuai Xue <xueshuai@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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (2 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 03/10] riscv: ftrace: use the frame pointer as the function graph return-address cookie Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:54 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 05/10] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue ` (6 subsequent siblings) 10 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Reliable frame-pointer unwinding needs an explicit way to identify exception boundaries and the final entry frame. The existing unwinder infers those boundaries from return addresses, which is too loose for a future reliable unwinder. Add a small metadata frame record to pt_regs and initialize it on exception entry, kernel stack overflow, kernel thread fork, user fork, and early idle task setup. The record uses a zero {fp, ra} sentinel plus a type field so a later unwinder can distinguish a final user-to-kernel boundary from a nested kernel pt_regs boundary. This follows the arm64 metadata frame-record model, adapted to the RISC-V {fp, ra} frame record convention. The metadata is established at the RISC-V entry boundaries that need an explicit unwind marker: * exception entry clears the metadata {fp, ra} pair and uses SPP (or MPP in M-mode) to record whether the pt_regs frame is the final user-to-kernel boundary or a nested kernel boundary; * the kernel stack overflow path builds a nested pt_regs metadata record on the overflow stack so an unwinder can resume from the pre-overflow s0 saved in PT_S0; * _start_kernel builds the init task's final metadata record, while the secondary CPU path sets up s0 before smp_callin() so idle-task unwinding does not inherit an undefined caller frame; * copy_thread creates matching final metadata records for new kernel and user tasks, and keeps s0 available for the frame-pointer chain. Keep the embedded metadata-record field offsets distinct from the s0-relative STACKFRAME_* offsets used by call_on_irq_stack(), because the latter describe a frame record relative to s0 rather than to the record base. These changes keep s0 reserved for the frame-pointer chain at task and exception boundaries. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/include/asm/ptrace.h | 9 ++++ arch/riscv/include/asm/stacktrace/frame.h | 53 +++++++++++++++++++++++ arch/riscv/kernel/asm-offsets.c | 6 +++ arch/riscv/kernel/entry.S | 39 ++++++++++++++++- arch/riscv/kernel/head.S | 23 ++++++++++ arch/riscv/kernel/process.c | 33 +++++++++++++- 6 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/frame.h diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h index addc8188152f..4b9b0f279214 100644 --- a/arch/riscv/include/asm/ptrace.h +++ b/arch/riscv/include/asm/ptrace.h @@ -8,6 +8,7 @@ #include <uapi/asm/ptrace.h> #include <asm/csr.h> +#include <asm/stacktrace/frame.h> #include <linux/compiler.h> #ifndef __ASSEMBLER__ @@ -53,6 +54,14 @@ struct pt_regs { unsigned long cause; /* a0 value before the syscall */ unsigned long orig_a0; + + /* + * This frame record is entirely zeroed on exception entry, allowing the + * unwinder to identify exception boundaries. The type field encodes + * whether the exception was taken from user (FINAL) or kernel (PT_REGS) + * mode. + */ + struct frame_record_meta stackframe; }; #define PTRACE_SYSEMU 0x1f diff --git a/arch/riscv/include/asm/stacktrace/frame.h b/arch/riscv/include/asm/stacktrace/frame.h new file mode 100644 index 000000000000..5720a6c65fe8 --- /dev/null +++ b/arch/riscv/include/asm/stacktrace/frame.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_RISCV_STACKTRACE_FRAME_H +#define __ASM_RISCV_STACKTRACE_FRAME_H + +/* + * See: arch/arm64/include/asm/stacktrace/frame.h for the reference + * implementation. + */ + +/* + * - FRAME_META_TYPE_NONE + * + * This value is reserved. + * + * - FRAME_META_TYPE_FINAL + * + * The record is the last entry on the stack. + * Unwinding should terminate successfully. + * + * - FRAME_META_TYPE_PT_REGS + * + * The record is embedded within a struct pt_regs, recording the registers at + * an arbitrary point in time. + * Unwinding should consume pt_regs::epc, followed by pt_regs::ra. + * + * Note: all other values are reserved and should result in unwinding + * terminating with an error. + */ +#define FRAME_META_TYPE_NONE 0 +#define FRAME_META_TYPE_FINAL 1 +#define FRAME_META_TYPE_PT_REGS 2 + +#ifndef __ASSEMBLER__ +/* + * A standard RISC-V frame record. + */ +struct frame_record { + unsigned long fp; + unsigned long ra; +}; + +/* + * A metadata frame record indicating a special unwind. + * The record::{fp,ra} fields must be zero to indicate the presence of + * metadata. + */ +struct frame_record_meta { + struct frame_record record; + unsigned long type; +}; +#endif /* __ASSEMBLER__ */ + +#endif /* __ASM_RISCV_STACKTRACE_FRAME_H */ diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c index a75f0cfea1e9..d0eaf2c902fb 100644 --- a/arch/riscv/kernel/asm-offsets.c +++ b/arch/riscv/kernel/asm-offsets.c @@ -131,6 +131,9 @@ void asm_offsets(void) OFFSET(PT_BADADDR, pt_regs, badaddr); OFFSET(PT_CAUSE, pt_regs, cause); + DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe)); + DEFINE(S_STACKFRAME_TYPE, offsetof(struct pt_regs, stackframe.type)); + OFFSET(SUSPEND_CONTEXT_REGS, suspend_context, regs); OFFSET(HIBERN_PBE_ADDR, pbe, address); @@ -503,6 +506,9 @@ void asm_offsets(void) DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN)); DEFINE(STACKFRAME_FP, offsetof(struct stackframe, fp) - sizeof(struct stackframe)); DEFINE(STACKFRAME_RA, offsetof(struct stackframe, ra) - sizeof(struct stackframe)); + DEFINE(STACKFRAME_RECORD_SIZE, sizeof(struct frame_record)); + OFFSET(FRAME_RECORD_FP, frame_record, fp); + OFFSET(FRAME_RECORD_RA, frame_record, ra); #ifdef CONFIG_FUNCTION_TRACER DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S index 61b7a7716697..4c69132d5978 100644 --- a/arch/riscv/kernel/entry.S +++ b/arch/riscv/kernel/entry.S @@ -11,6 +11,7 @@ #include <asm/asm.h> #include <asm/csr.h> #include <asm/scs.h> +#include <asm/stacktrace/frame.h> #include <asm/unistd.h> #include <asm/page.h> #include <asm/thread_info.h> @@ -198,6 +199,27 @@ SYM_CODE_START(handle_exception) REG_S s4, PT_CAUSE(sp) REG_S s5, PT_TP(sp) + /* + * Create a metadata frame record. The unwinder will use this to + * identify and unwind exception boundaries. + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) /* stackframe.record.fp = 0 */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) /* stackframe.record.ra = 0 */ +#ifdef CONFIG_RISCV_M_MODE + li t0, SR_MPP + and t0, s1, t0 +#else + andi t0, s1, SR_SPP +#endif + bnez t0, 1f + li t0, FRAME_META_TYPE_FINAL + j 2f +1: + li t0, FRAME_META_TYPE_PT_REGS +2: + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + /* * Set the scratch register to 0, so that if a recursive exception * occurs, the exception vector knows it came from the kernel @@ -354,6 +376,19 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow) REG_S s3, PT_BADADDR(sp) REG_S s4, PT_CAUSE(sp) REG_S s5, PT_TP(sp) + + /* + * Create a metadata frame record for the overflow pt_regs. The + * overflow path is entered from kernel context, so this is a nested + * pt_regs boundary and the unwinder can resume from the pre-overflow + * frame pointer saved in PT_S0. + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) + li t0, FRAME_META_TYPE_PT_REGS + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + move a0, sp tail handle_bad_stack SYM_CODE_END(handle_kernel_stack_overflow) @@ -362,8 +397,8 @@ ASM_NOKPROBE(handle_kernel_stack_overflow) SYM_CODE_START(ret_from_fork_kernel_asm) call schedule_tail - move a0, s1 /* fn_arg */ - move a1, s0 /* fn */ + move a0, s3 /* fn_arg */ + move a1, s2 /* fn */ move a2, sp /* pt_regs */ call ret_from_fork_kernel j ret_from_exception diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index 1631fd6bc786..887483b52d33 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -14,6 +14,7 @@ #include <asm/hwcap.h> #include <asm/image.h> #include <asm/scs.h> +#include <asm/stacktrace/frame.h> #include <asm/usercfi.h> #include <asm/alternative.h> #include <asm/errata_list.h> @@ -179,6 +180,14 @@ secondary_start_sbi: REG_S a0, (a1) 1: #endif + + /* + * Set up the frame pointer for the secondary idle task so reliable + * stack unwinding terminates at the metadata frame in task_pt_regs(). + * Without this, the first frame records can inherit an undefined caller + * fp and unwind past smp_callin() into .Lsecondary_park. + */ + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE scs_load_current call smp_callin #endif /* CONFIG_SMP */ @@ -307,6 +316,20 @@ SYM_CODE_START(_start_kernel) la tp, init_task la sp, init_thread_union + THREAD_SIZE addi sp, sp, -PT_SIZE_ON_STACK + + /* + * Set up a metadata frame record for the init task so that + * the unwinder can identify the outermost frame by its + * {fp, ra} = {0, 0} sentinel at the bottom of pt_regs. + * fp/s0 points above the metadata record (RISC-V + * convention). + */ + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) + li t0, FRAME_META_TYPE_FINAL + REG_S t0, S_STACKFRAME_TYPE(sp) + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE + #if defined(CONFIG_RISCV_SBI) && defined(CONFIG_RISCV_USER_CFI) li a7, SBI_EXT_FWFT li a6, SBI_EXT_FWFT_SET diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 34c1d498d1ca..223e851914d0 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -258,8 +258,23 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) /* Supervisor/Machine, irqs on: */ childregs->status = SR_PP | SR_PIE; - p->thread.s[0] = (unsigned long)args->fn; - p->thread.s[1] = (unsigned long)args->fn_arg; + /* + * Set up a metadata frame record at the bottom of the + * stack for the unwinder. Use FRAME_META_TYPE_FINAL + * since this is the outermost kernel entry for the new + * task. The frame_record::{fp,ra} are already zero from + * memset(). + * + * fp/s0 points above the metadata record (RISC-V + * convention). fn and fn_arg are passed via s2/s3, + * keeping s0 available for the frame pointer chain. + */ + childregs->stackframe.type = FRAME_META_TYPE_FINAL; + + p->thread.s[0] = (unsigned long)(&childregs->stackframe) + + sizeof(struct frame_record); + p->thread.s[2] = (unsigned long)args->fn; + p->thread.s[3] = (unsigned long)args->fn_arg; p->thread.ra = (unsigned long)ret_from_fork_kernel_asm; } else { /* allocate new shadow stack if needed. In case of CLONE_VM we have to */ @@ -278,6 +293,20 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) if (clone_flags & CLONE_SETTLS) childregs->tp = tls; childregs->a0 = 0; /* Return value of fork() */ + + /* + * Set up the unwind boundary: ensure the metadata + * frame record has its {fp,ra} sentinel zeroed and + * point fp/s0 above the metadata record. Mark it as + * FINAL since this is the outermost kernel entry for + * the new task. + */ + childregs->stackframe.record.fp = 0; + childregs->stackframe.record.ra = 0; + childregs->stackframe.type = FRAME_META_TYPE_FINAL; + p->thread.s[0] = (unsigned long)(&childregs->stackframe) + + sizeof(struct frame_record); + p->thread.ra = (unsigned long)ret_from_fork_user_asm; } p->thread.riscv_v_flags = 0; -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata 2026-11-10 1:37 ` [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata Shuai Xue @ 2026-11-10 1:54 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-11-10 1:54 UTC (permalink / raw) To: Shuai Xue, palmer, pjw, aou Cc: alex, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, jkchen, linux-trace-kernel, linux-perf-users, live-patching Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] riscv: entry: use call instead of tail for handle_bad_stack() -- commit ed05329452fecde9e16a00910af1efec91f2922b Author: Wang Han <wanghan@linux.alibaba.com> riscv: stacktrace: Add frame record metadata This commit adds a metadata frame record to struct pt_regs and initializes it across exception, overflow, boot, and task-creation entry boundaries for frame-pointer stack unwinding. > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > index 61b7a77166971..4c69132d59785 100644 > --- a/arch/riscv/kernel/entry.S > +++ b/arch/riscv/kernel/entry.S [ ... ] > @@ -354,6 +376,19 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow) > REG_S s3, PT_BADADDR(sp) > REG_S s4, PT_CAUSE(sp) > REG_S s5, PT_TP(sp) > + > + /* > + * Create a metadata frame record for the overflow pt_regs. The > + * overflow path is entered from kernel context, so this is a nested > + * pt_regs boundary and the unwinder can resume from the pre-overflow > + * frame pointer saved in PT_S0. > + */ > + REG_S zero, (S_STACKFRAME + FRAME_RECORD_FP)(sp) > + REG_S zero, (S_STACKFRAME + FRAME_RECORD_RA)(sp) > + li t0, FRAME_META_TYPE_PT_REGS > + REG_S t0, S_STACKFRAME_TYPE(sp) > + addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE > + > move a0, sp > tail handle_bad_stack > SYM_CODE_END(handle_kernel_stack_overflow) [Severity: Medium] Should this use "call handle_bad_stack" instead of "tail handle_bad_stack"? Because tail does not update ra, handle_bad_stack()'s prologue saves a frame record on the overflow stack that pairs the new s0 (pointing at the overflow pt_regs metadata record) with the interrupted pre-overflow ra. When panic() unwinds from handle_bad_stack(), the unwinder consumes regs->ra before reaching the pt_regs metadata record, and if that return address was hooked by function_graph or kretprobes, recovering it with the overflow-stack fp fails and aborts the stack trace before reaching the overflowed task stack. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110013750.36800-1-xueshuai@linux.alibaba.com?part=4 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 05/10] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (3 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 06/10] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue ` (5 subsequent siblings) 10 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> KASAN records stack traces for every alloc/free, which means it walks the unwinder very frequently. Instrumenting the stack trace collection code itself adds substantial overhead and makes the traces themselves noisier. KCOV instruments every basic-block edge. The unwinder is a hot path, especially with KASAN enabled, so KCOV instrumentation has the same kind of cost and noise problem here. Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86 treatment of its stack unwinding code. RISC-V keeps the relevant unwinder code in stacktrace.o, so a single translation-unit annotation covers the equivalent scope. This is a prerequisite preference for the upcoming reliable unwinder, but the change is valid on its own. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index f54c336bf547..c4c0f4d73368 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -44,6 +44,12 @@ CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) endif +# When KASAN is enabled, a stack trace is recorded for every alloc/free, which +# can significantly impact performance. Avoid instrumenting the stack trace +# collection code to minimize this impact. +KASAN_SANITIZE_stacktrace.o := n +KCOV_INSTRUMENT_stacktrace.o := n + always-$(KBUILD_BUILTIN) += vmlinux.lds obj-y += head.o -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v7 06/10] riscv: ftrace: always preserve s0 in dynamic ftrace register frame 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (4 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 05/10] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 07/10] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue ` (4 subsequent siblings) 10 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> struct __arch_ftrace_regs declares s0 unconditionally, and both ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST (CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before, so any callback going through ftrace_partial_regs() saw a garbage regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this has not bitten in practice. Save and restore s0 unconditionally in the dynamic ftrace ABI register frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace path in line with the static _mcount path (mcount.S SAVE_ABI_STATE already saves s0 unconditionally), and matches the frame layout already documented in the comment above SAVE_ABI_REGS. It is also a prerequisite for the upcoming reliable unwinder, which reads ftrace_regs_get_frame_pointer(fregs) directly. The cost is one extra REG_S/REG_L pair per traced call, negligible compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK already reserved the slot, so no extra stack space is used. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/mcount-dyn.S | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index 082fe0b0e3c0..26c55fba8fec 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -85,9 +85,7 @@ addi sp, sp, -FREGS_SIZE_ON_STACK REG_S t0, FREGS_EPC(sp) REG_S x1, FREGS_RA(sp) -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST REG_S x8, FREGS_S0(sp) -#endif REG_S x6, FREGS_T1(sp) #ifdef CONFIG_CC_IS_CLANG REG_S x7, FREGS_T2(sp) @@ -113,9 +111,7 @@ .macro RESTORE_ABI_REGS REG_L t0, FREGS_EPC(sp) REG_L x1, FREGS_RA(sp) -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST REG_L x8, FREGS_S0(sp) -#endif REG_L x6, FREGS_T1(sp) #ifdef CONFIG_CC_IS_CLANG REG_L x7, FREGS_T2(sp) -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v7 07/10] riscv: stacktrace: introduce stack-bound tracking helpers 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (5 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 06/10] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 1:37 ` [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue ` (3 subsequent siblings) 10 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> A reliable unwinder needs to validate that every frame record it reads is fully contained in a known kernel stack, and it needs to refuse to walk back into a stack it has already left. Add the building blocks for that: * struct stack_info / struct unwind_state in a new asm/stacktrace/common.h, modelled on the arm64 reference implementation. * stackinfo_get_irq() / stackinfo_get_task() / stackinfo_get_overflow() plus the corresponding on_*_stack() predicates in asm/stacktrace.h, so callers can ask "is this object on stack X?" by stack kind rather than open-coded address arithmetic. * unwind_init_common(), unwind_find_stack() and unwind_consume_stack() helpers that enforce the forward-progress-only invariant required for reliability. No existing user is wired up to these helpers in this commit; the unwinder switch comes in a follow-up. The header changes leave on_thread_stack() with the same semantics as before, just expressed in terms of the new helpers. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/include/asm/stacktrace.h | 67 ++++++++- arch/riscv/include/asm/stacktrace/common.h | 159 +++++++++++++++++++++ 2 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/common.h diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h index b1495a7e06ce..df4d022f6430 100644 --- a/arch/riscv/include/asm/stacktrace.h +++ b/arch/riscv/include/asm/stacktrace.h @@ -3,8 +3,13 @@ #ifndef _ASM_RISCV_STACKTRACE_H #define _ASM_RISCV_STACKTRACE_H +#include <linux/percpu.h> #include <linux/sched.h> +#include <linux/sched/task_stack.h> + +#include <asm/irq_stack.h> #include <asm/ptrace.h> +#include <asm/stacktrace/common.h> struct stackframe { unsigned long fp; @@ -16,14 +21,72 @@ extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *re extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl); -static inline bool on_thread_stack(void) +/* + * IRQ stack accessors + */ +#ifdef CONFIG_IRQ_STACKS +static inline struct stack_info stackinfo_get_irq(void) +{ + unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); + unsigned long high = low + IRQ_STACK_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} + +static inline bool on_irq_stack(unsigned long sp, unsigned long size) +{ + struct stack_info info = stackinfo_get_irq(); + + return stackinfo_on_stack(&info, sp, size); +} +#endif /* CONFIG_IRQ_STACKS */ + +/* + * Task stack accessors + */ +static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk) { - return !(((unsigned long)(current->stack) ^ current_stack_pointer) & ~(THREAD_SIZE - 1)); + unsigned long low = (unsigned long)task_stack_page(tsk); + unsigned long high = low + THREAD_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} + +static inline bool on_task_stack(const struct task_struct *tsk, + unsigned long sp, unsigned long size) +{ + struct stack_info info = stackinfo_get_task(tsk); + + return stackinfo_on_stack(&info, sp, size); } +/* + * Cast is necessary since current->stack is an opaque ptr. + */ +#define on_thread_stack() (on_task_stack(current, current_stack_pointer, 1)) +/* + * Overflow stack accessors + */ #ifdef CONFIG_VMAP_STACK DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack); + +static inline struct stack_info stackinfo_get_overflow(void) +{ + unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack); + unsigned long high = low + OVERFLOW_STACK_SIZE; + + return (struct stack_info) { + .low = low, + .high = high, + }; +} #endif /* CONFIG_VMAP_STACK */ #endif /* _ASM_RISCV_STACKTRACE_H */ diff --git a/arch/riscv/include/asm/stacktrace/common.h b/arch/riscv/include/asm/stacktrace/common.h new file mode 100644 index 000000000000..360a26e34349 --- /dev/null +++ b/arch/riscv/include/asm/stacktrace/common.h @@ -0,0 +1,159 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * RISC-V common stack unwinder types and helpers. + * + * See: arch/arm64/include/asm/stacktrace/common.h for the reference + * implementation. + * + * Copyright (C) 2026 + */ +#ifndef __ASM_RISCV_STACKTRACE_COMMON_H +#define __ASM_RISCV_STACKTRACE_COMMON_H + +#include <linux/compiler.h> +#include <linux/errno.h> +#include <linux/types.h> + +#include <asm/stacktrace/frame.h> + +/** + * struct stack_info - describes the bounds of a stack. + * + * @low: The lowest valid address on the stack. + * @high: The highest valid address on the stack. + */ +struct stack_info { + unsigned long low; + unsigned long high; +}; + +/** + * struct unwind_state - state used for robust unwinding. + * + * @fp: The fp value in the frame record (or the real fp). + * @pc: The ra value in the frame record (or the real ra). + * + * @stack: The stack currently being unwound. + * @stacks: An array of stacks which can be unwound. + * @nr_stacks: The number of stacks in @stacks. + */ +struct unwind_state { + unsigned long fp; + unsigned long pc; + + struct stack_info stack; + struct stack_info *stacks; + int nr_stacks; +}; + +/** + * stackinfo_get_unknown() - Get an unknown stack_info. + * + * Return: a stack_info with low and high set to 0. + */ +static inline struct stack_info stackinfo_get_unknown(void) +{ + return (struct stack_info) { + .low = 0, + .high = 0, + }; +} + +/** + * stackinfo_on_stack() - Check whether an object is fully within a stack. + * + * @info: The stack to check against. + * @sp: The base address of the object. + * @size: The size of the object. + * + * Return: true if the object is fully contained within the stack. + */ +static inline bool stackinfo_on_stack(const struct stack_info *info, + unsigned long sp, unsigned long size) +{ + if (!info->low) + return false; + + if (sp < info->low || sp + size < sp || sp + size > info->high) + return false; + + return true; +} + +/** + * unwind_init_common() - Initialize the common parts of the unwind state. + * + * @state: the unwind state to initialize. + */ +static inline void unwind_init_common(struct unwind_state *state) +{ + state->stack = stackinfo_get_unknown(); +} + +/** + * unwind_find_stack() - Find the accessible stack which entirely contains an + * object. + * + * @state: the current unwind state. + * @sp: the base address of the object. + * @size: the size of the object. + * + * Return: a pointer to the relevant stack_info if found; NULL otherwise. + */ +static inline struct stack_info *unwind_find_stack(struct unwind_state *state, + unsigned long sp, + unsigned long size) +{ + struct stack_info *info = &state->stack; + + if (stackinfo_on_stack(info, sp, size)) + return info; + + for (int i = 0; i < state->nr_stacks; i++) { + info = &state->stacks[i]; + if (stackinfo_on_stack(info, sp, size)) + return info; + } + + return NULL; +} + +/** + * unwind_consume_stack() - Update stack boundaries so that future unwind steps + * cannot consume this object again. + * + * @state: the current unwind state. + * @info: the stack_info of the stack containing the object. + * @sp: the base address of the object. + * @size: the size of the object. + * + * Stack transitions are strictly one-way, and once we've + * transitioned from one stack to another, it's never valid to + * unwind back to the old stack. + * + * Note that stacks can nest in several valid orders, e.g. + * + * TASK -> IRQ -> OVERFLOW + * + * ... so we do not check the specific order of stack + * transitions. + */ +static inline void unwind_consume_stack(struct unwind_state *state, + struct stack_info *info, + unsigned long sp, + unsigned long size) +{ + struct stack_info tmp; + + tmp = *info; + *info = stackinfo_get_unknown(); + state->stack = tmp; + + /* + * Future unwind steps can only consume stack above this frame record. + * Update the current stack to start immediately above it. + */ + state->stack.low = sp + size; +} + +#endif /* __ASM_RISCV_STACKTRACE_COMMON_H */ -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (6 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 07/10] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 2:20 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue ` (2 subsequent siblings) 10 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Replace the open-coded frame-pointer walker in arch_stack_walk() with a robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c and retargeted to the RISC-V {fp, ra} frame record convention. The new walker tracks stack bounds, consumes frame records monotonically, understands the metadata pt_regs records added in the previous frame record metadata patch, and recovers return addresses replaced by function graph tracing and kretprobes. Function graph recovery matches the caller's frame pointer, the convention set up by the preceding ftrace cookie patch. This commit introduces arch_stack_walk_reliable() but does not yet select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig patch so this commit can be reviewed and bisected as a pure unwinder replacement. Until that Kconfig change lands, livepatch is not yet enabled and arch_stack_walk_reliable() has no in-tree caller. Two related callers are updated to keep the same frame-record assumptions everywhere: * Perf callchains: route kernel callchain collection through arch_stack_walk() so perf sees the same frame-pointer unwind behaviour as dump_stack() and the upcoming livepatch path. * dump_backtrace() / __get_wchan() / show_stack(): these now go through arch_stack_walk(); the explicit "Call Trace:" header is moved into dump_backtrace() to preserve the original output. For !CONFIG_FRAME_POINTER builds the stack-scanning fallback walker follows the approach of the first patch of this series: the walker is extracted into an __always_inline __walk_stackframe() helper that arch_stack_walk() calls directly, so the first reported entry is the return into arch_stack_walk()'s caller. walk_stackframe() remains as a wrapper for its existing callers. Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/kernel/perf_callchain.c | 2 +- arch/riscv/kernel/stacktrace.c | 588 +++++++++++++++++++++++------ 2 files changed, 475 insertions(+), 115 deletions(-) diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c index b465bc9eb870..436af96ea59c 100644 --- a/arch/riscv/kernel/perf_callchain.c +++ b/arch/riscv/kernel/perf_callchain.c @@ -44,5 +44,5 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, return; } - walk_stackframe(NULL, regs, fill_callchain, entry); + arch_stack_walk(fill_callchain, entry, NULL, regs); } diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c index a5abb705d43b..490092c28d61 100644 --- a/arch/riscv/kernel/stacktrace.c +++ b/arch/riscv/kernel/stacktrace.c @@ -11,114 +11,16 @@ #include <linux/sched/task_stack.h> #include <linux/stacktrace.h> #include <linux/ftrace.h> +#include <linux/kprobes.h> +#include <linux/llist.h> #include <asm/stacktrace.h> -#ifdef CONFIG_FRAME_POINTER - /* - * This disables KASAN checking when reading a value from another task's stack, - * since the other task could be running on another CPU and could have poisoned - * the stack in the meantime. + * Non-frame-pointer fallback unwinder. + * Only compiled when CONFIG_FRAME_POINTER is not enabled. */ -#define READ_ONCE_TASK_STACK(task, x) \ -({ \ - unsigned long val; \ - unsigned long addr = x; \ - if ((task) == current) \ - val = READ_ONCE(addr); \ - else \ - val = READ_ONCE_NOCHECK(addr); \ - val; \ -}) - -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) -{ - unsigned long low, high; - - low = sp + sizeof(struct stackframe); - high = ALIGN(sp, THREAD_SIZE); - - return !(fp < low || fp > high || fp & 0x07); -} - -/* - * Always inlined so that the frame the walk starts from is the caller's own, - * and the first entry reported for the current task is the caller's return - * address. arch_stack_walk() relies on that to report the same frames as - * the other architectures, see the comment there. - */ -static __always_inline void __walk_stackframe(struct task_struct *task, - struct pt_regs *regs, - bool (*fn)(void *, unsigned long), - void *arg) -{ - unsigned long fp, sp, pc; - int graph_idx = 0; - int level = 0; - - if (regs) { - fp = frame_pointer(regs); - sp = user_stack_pointer(regs); - pc = instruction_pointer(regs); - } else if (task == NULL || task == current) { - fp = (unsigned long)__builtin_frame_address(0); - sp = current_stack_pointer; - /* Placeholder for the frame the walk starts from, never reported. */ - pc = (unsigned long)walk_stackframe; - level = -1; - } else { - /* task blocked in __switch_to */ - fp = task->thread.s[0]; - sp = task->thread.sp; - pc = task->thread.ra; - } - - for (;;) { - struct stackframe *frame; - - if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc)))) - break; - - if (unlikely(!fp_is_valid(fp, sp))) - break; - - /* Unwind stack frame */ - frame = (struct stackframe *)fp - 1; - sp = fp; - if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp)) { - /* We hit function where ra is not saved on the stack */ - fp = frame->ra; - pc = regs->ra; - } 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, - (void *)fp); - if (pc >= (unsigned long)handle_exception && - pc < (unsigned long)&ret_from_exception_end) { - if (unlikely(!fn(arg, pc))) - break; - - pc = ((struct pt_regs *)sp)->epc; - fp = ((struct pt_regs *)sp)->s0; - } - } - - } -} - -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg) -{ - __walk_stackframe(task, regs, fn, arg); -} - -#else /* !CONFIG_FRAME_POINTER */ +#ifndef CONFIG_FRAME_POINTER static __always_inline void __walk_stackframe(struct task_struct *task, struct pt_regs *regs, @@ -151,13 +53,18 @@ static __always_inline void __walk_stackframe(struct task_struct *task, } } -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg) +void notrace walk_stackframe(struct task_struct *task, + struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) { __walk_stackframe(task, regs, fn, arg); } -#endif /* CONFIG_FRAME_POINTER */ +#endif /* !CONFIG_FRAME_POINTER */ + +/* + * Common trace helpers. + * These are used by both the FP (kunwind) and non-FP (walk_stackframe) paths. + */ static bool print_trace_address(void *arg, unsigned long pc) { @@ -170,12 +77,12 @@ static bool print_trace_address(void *arg, unsigned long pc) noinline void dump_backtrace(struct pt_regs *regs, struct task_struct *task, const char *loglvl) { - walk_stackframe(task, regs, print_trace_address, (void *)loglvl); + printk("%sCall Trace:\n", loglvl); + arch_stack_walk(print_trace_address, (void *)loglvl, task, regs); } void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl) { - pr_cont("%sCall Trace:\n", loglvl); dump_backtrace(NULL, task, loglvl); } @@ -195,24 +102,477 @@ unsigned long __get_wchan(struct task_struct *task) if (!try_get_task_stack(task)) return 0; - walk_stackframe(task, NULL, save_wchan, &pc); + arch_stack_walk(save_wchan, &pc, task, NULL); put_task_stack(task); return pc; } +/* + * Frame-pointer-based kernel unwind infrastructure. + * Only compiled when CONFIG_FRAME_POINTER is enabled. + * + * See: arch/arm64/kernel/stacktrace.c for the reference implementation. + */ +#ifdef CONFIG_FRAME_POINTER + +/* + * Per-cpu stacks are only accessible when unwinding the current task in a + * non-preemptible context. + */ +#define STACKINFO_CPU(task, name) \ + ({ \ + (((task) == current) && !preemptible()) \ + ? stackinfo_get_##name() \ + : stackinfo_get_unknown(); \ + }) + +enum kunwind_source { + KUNWIND_SOURCE_UNKNOWN, + KUNWIND_SOURCE_FRAME, + KUNWIND_SOURCE_CALLER, + KUNWIND_SOURCE_TASK, + KUNWIND_SOURCE_REGS_PC, +}; + +union unwind_flags { + unsigned long all; + struct { + unsigned long fgraph : 1, + kretprobe : 1; + }; +}; + +/* + * Kernel unwind state + * + * @common: Common unwind state. + * @task: The task being unwound. + * @graph_idx: Used by ftrace_graph_ret_addr() for optimized stack unwinding. + * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance + * associated with the most recently encountered replacement ra + * value. + */ +struct kunwind_state { + struct unwind_state common; + struct task_struct *task; + int graph_idx; +#ifdef CONFIG_KRETPROBES + struct llist_node *kr_cur; +#endif + enum kunwind_source source; + union unwind_flags flags; + struct pt_regs *regs; +}; + +static __always_inline void +kunwind_init(struct kunwind_state *state, + struct task_struct *task) +{ + unwind_init_common(&state->common); + state->task = task; + state->source = KUNWIND_SOURCE_UNKNOWN; + state->flags.all = 0; + state->regs = NULL; +} + +/* + * Start an unwind from a pt_regs. + * + * The unwind will begin at the PC within the regs. + * + * The regs must be on a stack currently owned by the calling task. + */ +static __always_inline void +kunwind_init_from_regs(struct kunwind_state *state, + struct pt_regs *regs) +{ + kunwind_init(state, current); + + state->regs = regs; + state->common.fp = frame_pointer(regs); + state->common.pc = instruction_pointer(regs); + state->source = KUNWIND_SOURCE_REGS_PC; +} + +/* + * Start an unwind from a caller. + * + * The unwind will begin at the caller of whichever function this is inlined + * into. + * + * The function which invokes this must be noinline. + */ +static __always_inline void +kunwind_init_from_caller(struct kunwind_state *state) +{ + unsigned long fp = (unsigned long)__builtin_frame_address(0); + struct frame_record *record = (struct frame_record *)fp - 1; + + kunwind_init(state, current); + + state->common.fp = READ_ONCE(record->fp); + state->common.pc = READ_ONCE(record->ra); + state->source = KUNWIND_SOURCE_CALLER; +} + +/* + * Start an unwind from a blocked task. + * + * The unwind will begin at the blocked task's saved PC (i.e. the caller of + * __switch_to). + * + * The caller should ensure the task is blocked in __switch_to for the + * duration of the unwind, or the unwind will be bogus. It is never valid to + * call this for the current task. + */ +static __always_inline void +kunwind_init_from_task(struct kunwind_state *state, + struct task_struct *task) +{ + kunwind_init(state, task); + + state->common.fp = task->thread.s[0]; + state->common.pc = task->thread.ra; + state->source = KUNWIND_SOURCE_TASK; +} + +static __always_inline int +kunwind_recover_return_address(struct kunwind_state *state) +{ +#ifdef CONFIG_FUNCTION_GRAPH_TRACER + if (state->task->ret_stack && + state->common.pc == (unsigned long)return_to_handler) { + unsigned long orig_pc; + + orig_pc = ftrace_graph_ret_addr(state->task, &state->graph_idx, + state->common.pc, + (void *)state->common.fp); + if (state->common.pc == orig_pc) { + WARN_ON_ONCE(state->task == current); + return -EINVAL; + } + state->common.pc = orig_pc; + state->flags.fgraph = 1; + } +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ + +#ifdef CONFIG_KRETPROBES + if (is_kretprobe_trampoline(state->common.pc)) { + unsigned long orig_pc; + + orig_pc = kretprobe_find_ret_addr(state->task, + (void *)state->common.fp, + &state->kr_cur); + if (!orig_pc) + return -EINVAL; + state->common.pc = orig_pc; + state->flags.kretprobe = 1; + } +#endif /* CONFIG_KRETPROBES */ + + return 0; +} + +/* + * When we reach an exception boundary marked by a metadata frame record, + * extract pt_regs from the stack and continue unwinding from the saved + * context (epc and s0/fp). + * + * On RISC-V, fp points above the metadata record, so the record's + * frame_record portion is at fp - sizeof(struct frame_record). + */ +static __always_inline int +kunwind_next_regs_pc(struct kunwind_state *state) +{ + struct stack_info *info; + unsigned long fp = state->common.fp; + struct pt_regs *regs; + + regs = container_of((unsigned long *)(fp - sizeof(struct frame_record)), + struct pt_regs, stackframe.record.fp); + + info = unwind_find_stack(&state->common, (unsigned long)regs, + sizeof(*regs)); + if (!info) + return -EINVAL; + + unwind_consume_stack(&state->common, info, (unsigned long)regs, + sizeof(*regs)); + + state->regs = regs; + state->common.pc = regs->epc; + state->common.fp = frame_pointer(regs); + state->source = KUNWIND_SOURCE_REGS_PC; + return 0; +} + +/* + * Handle a metadata frame record embedded in pt_regs. + * + * On RISC-V, fp points above the record (fp = metadata + 16), so the + * frame_record_meta starts at fp - sizeof(struct frame_record). + * + * FRAME_META_TYPE_FINAL: This is the outermost exception entry + * (user -> kernel). Unwinding terminates successfully. + * FRAME_META_TYPE_PT_REGS: This is a nested exception entry + * (kernel -> kernel). Continue unwinding from the saved context. + */ +static __always_inline int +kunwind_next_frame_record_meta(struct kunwind_state *state) +{ + struct task_struct *tsk = state->task; + unsigned long fp = state->common.fp; + unsigned long meta_base = fp - sizeof(struct frame_record); + struct frame_record_meta *meta; + struct stack_info *info; + + info = unwind_find_stack(&state->common, meta_base, sizeof(*meta)); + if (!info) + return -EINVAL; + + meta = (struct frame_record_meta *)meta_base; + switch (READ_ONCE(meta->type)) { + case FRAME_META_TYPE_FINAL: + if (meta == &task_pt_regs(tsk)->stackframe) + return -ENOENT; + WARN_ON_ONCE(tsk == current); + return -EINVAL; + case FRAME_META_TYPE_PT_REGS: + return kunwind_next_regs_pc(state); + default: + WARN_ON_ONCE(tsk == current); + return -EINVAL; + } +} + +/* + * Unwind from one frame record to the next. + * + * On RISC-V, the frame record sits at fp - sizeof(struct frame_record), + * immediately below the address pointed to by fp/s0. This applies to both + * normal frame records and metadata frame records (embedded in pt_regs). + * + * A metadata record is identified by both fp and ra being zero in the + * frame_record portion, with a type value following at fp + 16. + */ +static __always_inline int +kunwind_next_frame_record(struct kunwind_state *state) +{ + unsigned long fp = state->common.fp; + struct frame_record *record; + struct stack_info *info; + unsigned long new_fp, new_pc; + unsigned long record_base; + + if (fp & 0x7) + return -EINVAL; + + record_base = fp - sizeof(*record); + + info = unwind_find_stack(&state->common, record_base, sizeof(*record)); + if (!info) + return -EINVAL; + + record = (struct frame_record *)record_base; + new_fp = READ_ONCE(record->fp); + new_pc = READ_ONCE(record->ra); + + if (!new_fp && !new_pc) + return kunwind_next_frame_record_meta(state); + + unwind_consume_stack(&state->common, info, record_base, + sizeof(*record)); + + state->common.fp = new_fp; + state->common.pc = new_pc; + state->source = KUNWIND_SOURCE_FRAME; + + return 0; +} + +/* + * Unwind from one frame record (A) to the next frame record (B). + * + * We terminate early if the location of B indicates a malformed chain of frame + * records (e.g. a cycle), determined based on the location and fp value of A + * and the location (but not the fp value) of B. + */ +static __always_inline int +kunwind_next(struct kunwind_state *state) +{ + int err; + + state->flags.all = 0; + + switch (state->source) { + case KUNWIND_SOURCE_FRAME: + case KUNWIND_SOURCE_CALLER: + case KUNWIND_SOURCE_TASK: + case KUNWIND_SOURCE_REGS_PC: + err = kunwind_next_frame_record(state); + break; + default: + err = -EINVAL; + } + + if (err) + return err; + + return kunwind_recover_return_address(state); +} + +typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie); + +static __always_inline int +do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state, + void *cookie) +{ + int ret; + + ret = kunwind_recover_return_address(state); + if (ret) + return ret; + + while (1) { + if (!consume_state(state, cookie)) + return -EINVAL; + ret = kunwind_next(state); + if (ret == -ENOENT) + return 0; + if (ret < 0) + return ret; + } +} + +static __always_inline int +kunwind_stack_walk(kunwind_consume_fn consume_state, + void *cookie, struct task_struct *task, + struct pt_regs *regs) +{ + struct task_struct *tsk = task ?: current; + struct stack_info stacks[] = { + stackinfo_get_task(tsk), +#ifdef CONFIG_IRQ_STACKS + STACKINFO_CPU(tsk, irq), +#endif +#ifdef CONFIG_VMAP_STACK + STACKINFO_CPU(tsk, overflow), +#endif + }; + struct kunwind_state state = { + .common = { + .stacks = stacks, + .nr_stacks = ARRAY_SIZE(stacks), + }, + }; + + if (regs) { + if (tsk != current) + return -EINVAL; + kunwind_init_from_regs(&state, regs); + } else if (tsk == current) { + kunwind_init_from_caller(&state); + } else { + kunwind_init_from_task(&state, tsk); + } + + return do_kunwind(&state, consume_state, cookie); +} + +struct kunwind_consume_entry_data { + stack_trace_consume_fn consume_entry; + void *cookie; +}; + +static __always_inline bool +arch_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) +{ + struct kunwind_consume_entry_data *data = cookie; + + return data->consume_entry(data->cookie, state->common.pc); +} + +static __always_inline bool +arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) +{ + /* + * At an exception boundary we can reliably consume the saved PC. We do + * not know whether ra was live when the exception was taken, and + * so we cannot perform the next unwind step reliably. + * + * All that matters is whether the *entire* unwind is reliable, so give + * up as soon as we hit an exception boundary. + */ + if (state->source == KUNWIND_SOURCE_REGS_PC) + return false; + + return arch_kunwind_consume_entry(state, cookie); +} + +#endif /* CONFIG_FRAME_POINTER */ + +/* + * arch_stack_walk - dual implementation. + * + * When CONFIG_FRAME_POINTER is enabled, uses the kunwind infrastructure for + * robust frame-pointer-based unwinding, consistent with arch_stack_walk_reliable. + * + * When CONFIG_FRAME_POINTER is disabled, falls back to the simple stack + * scan in the always-inlined __walk_stackframe(). + */ +#ifdef CONFIG_FRAME_POINTER + +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) +{ + struct kunwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); +} + +#else + /* * The generic code expects the first entry for the current task to be the * return address of arch_stack_walk() itself, the way x86 and arm64 report - * it, and stack_trace_save() sizes its skip count accordingly. Walking from - * inside a called walk_stackframe() adds that function's own frame on top, - * so inline the walker here and start from this frame instead. + * it, and stack_trace_save() sizes its skip count accordingly. Walking + * from inside a called walk_stackframe() adds that function's own frame + * on top, so call the always-inlined walker here and start from this + * frame instead. */ -noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, - struct task_struct *task, struct pt_regs *regs) +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) { __walk_stackframe(task, regs, consume_entry, cookie); } +#endif /* CONFIG_FRAME_POINTER */ + +/* + * Reliable stack walk for livepatch (CONFIG_FRAME_POINTER only). + */ +#ifdef CONFIG_FRAME_POINTER + +noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, + void *cookie, + struct task_struct *task) +{ + struct kunwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + return kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data, + task, NULL); +} + +#endif /* CONFIG_FRAME_POINTER */ + /* * Get the return address for a single stackframe and return a pointer to the * next frame tail. -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder 2026-11-10 1:37 ` [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue @ 2026-11-10 2:20 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-11-10 2:20 UTC (permalink / raw) To: Shuai Xue, palmer, pjw, aou Cc: alex, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, jkchen, linux-trace-kernel, linux-perf-users, live-patching Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [High] riscv: stacktrace: broken leaf-function unwind in kunwind_next_frame_record() - [High] riscv: stacktrace: broken unwind across ftrace_caller and return_to_handler - [Medium] riscv: stacktrace: bogus walk_stackframe entry in __walk_stackframe() - [Low] riscv: stacktrace: inaccurate metadata type offset comment -- commit 263985113a916e8cd08cc4485092812e49e28303 Author: Wang Han <wanghan@linux.alibaba.com> riscv: stacktrace: switch to frame-pointer based unwinder This commit replaces the open-coded frame-pointer stack walker with a kunwind state machine, adds arch_stack_walk_reliable(), and routes perf_callchain_kernel(), dump_backtrace(), and __get_wchan() through arch_stack_walk(). > diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c > index a5abb705d43ba..490092c28d612 100644 > --- a/arch/riscv/kernel/stacktrace.c > +++ b/arch/riscv/kernel/stacktrace.c [ ... ] > +/* > + * Unwind from one frame record to the next. > + * > + * On RISC-V, the frame record sits at fp - sizeof(struct frame_record), > + * immediately below the address pointed to by fp/s0. This applies to both > + * normal frame records and metadata frame records (embedded in pt_regs). > + * > + * A metadata record is identified by both fp and ra being zero in the > + * frame_record portion, with a type value following at fp + 16. > + */ [Severity: Low] This isn't a bug, but since fp points immediately above struct frame_record on RISC-V (meta_base = fp - sizeof(struct frame_record)), meta->type is located at fp rather than fp + 16. > +static __always_inline int > +kunwind_next_frame_record(struct kunwind_state *state) > +{ > + unsigned long fp = state->common.fp; > + struct frame_record *record; > + struct stack_info *info; > + unsigned long new_fp, new_pc; > + unsigned long record_base; > + > + if (fp & 0x7) > + return -EINVAL; > + > + record_base = fp - sizeof(*record); > + > + info = unwind_find_stack(&state->common, record_base, sizeof(*record)); > + if (!info) > + return -EINVAL; > + > + record = (struct frame_record *)record_base; > + new_fp = READ_ONCE(record->fp); > + new_pc = READ_ONCE(record->ra); > + > + if (!new_fp && !new_pc) > + return kunwind_next_frame_record_meta(state); > + > + unwind_consume_stack(&state->common, info, record_base, > + sizeof(*record)); > + > + state->common.fp = new_fp; > + state->common.pc = new_pc; > + state->source = KUNWIND_SOURCE_FRAME; > + > + return 0; > +} [Severity: High] Does unwinding from KUNWIND_SOURCE_REGS_PC corrupt the unwind and leak kernel stack addresses when an exception or interrupt hits a leaf function? On RISC-V with CONFIG_FRAME_POINTER=y, leaf functions do not push ra onto the stack: they save only the caller's s0 at fp - sizeof(unsigned long) (which aliases record->ra), leave the return address in regs->ra, and leave fp - 2 * sizeof(unsigned long) (record->fp) as uninitialized stack padding or local storage. The previous unwinder checked for this case, whereas kunwind_init_from_regs() and kunwind_next_regs_pc() store regs in state->regs without ever reading it back, and kunwind_next_frame_record() unconditionally reads record->ra as new_pc and record->fp as new_fp. Because __kernel_text_address() filtering is also removed and perf_callchain_kernel() now calls arch_stack_walk(), a PMU sample in a leaf function passes the caller's saved kernel stack pointer in record->ra to fill_callchain() -> perf_callchain_store(), exposing it to userspace, while regs->ra is lost and the uninitialized word in record->fp is used as the next frame pointer. [ ... ] > +static __always_inline bool > +arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie) > +{ > + /* > + * At an exception boundary we can reliably consume the saved PC. We do > + * not know whether ra was live when the exception was taken, and > + * so we cannot perform the next unwind step reliably. > + * > + * All that matters is whether the *entire* unwind is reliable, so give > + * up as soon as we hit an exception boundary. > + */ > + if (state->source == KUNWIND_SOURCE_REGS_PC) > + return false; > + > + return arch_kunwind_consume_entry(state, cookie); > +} [Severity: High] Can kunwind_next_frame_record() and arch_stack_walk_reliable() unwind across ftrace_caller and return_to_handler? Unlike arm64, RISC-V's ftrace trampolines do not maintain a valid struct frame_record chain at s0 - sizeof(struct frame_record): arch/riscv/kernel/mcount-dyn.S:SAVE_ABI_REGS { addi sp, sp, -FREGS_SIZE_ON_STACK REG_S t0, FREGS_EPC(sp) REG_S x1, FREGS_RA(sp) REG_S x8, FREGS_S0(sp) ... } arch/riscv/kernel/mcount.S:SAVE_RET_ABI_STATE { addi sp, sp, -FREGS_SIZE_ON_STACK REG_S ra, FREGS_RA(sp) REG_S s0, FREGS_S0(sp) REG_S a0, FREGS_A0(sp) REG_S a1, FREGS_A1(sp) addi s0, sp, FREGS_SIZE_ON_STACK } In ftrace_caller, SAVE_ABI_REGS saves ftrace_regs without updating s0 or creating a frame record, so a C tracer callback saves the traced function's caller s0 alongside a return address in ftrace_caller. Walking past the callback jumps straight from ftrace_caller to the caller of the traced function, silently skipping both the traced function and its immediate caller while arch_stack_walk_reliable() still succeeds. (Commit "riscv: ftrace: always preserve s0 in dynamic ftrace register frame" noted that the reliable unwinder would read ftrace_regs_get_frame_pointer(fregs) directly, but kunwind has no ftrace_regs handling.) In return_to_handler, SAVE_RET_ABI_STATE sets s0 to sp + FREGS_SIZE_ON_STACK after saving only ra, s0, a0, and a1. When unwinding past ftrace_return_to_handler(), kunwind_next_frame_record() reads s0 - sizeof(struct frame_record), which sits in the uninitialized upper words of the __arch_ftrace_regs allocation rather than at FREGS_S0(sp). [ ... ] > +/* > * The generic code expects the first entry for the current task to be the > * return address of arch_stack_walk() itself, the way x86 and arm64 report > - * it, and stack_trace_save() sizes its skip count accordingly. Walking from > - * inside a called walk_stackframe() adds that function's own frame on top, > - * so inline the walker here and start from this frame instead. > + * it, and stack_trace_save() sizes its skip count accordingly. Walking > + * from inside a called walk_stackframe() adds that function's own frame > + * on top, so call the always-inlined walker here and start from this > + * frame instead. > */ > -noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > - struct task_struct *task, struct pt_regs *regs) > +noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > + void *cookie, struct task_struct *task, > + struct pt_regs *regs) > { > __walk_stackframe(task, regs, consume_entry, cookie); > } [Severity: Medium] On !CONFIG_FRAME_POINTER builds, doesn't __walk_stackframe() still report walk_stackframe as the first entry when unwinding the current task? arch/riscv/kernel/stacktrace.c:__walk_stackframe() { ... } else if (task == NULL || task == current) { sp = current_stack_pointer; pc = (unsigned long)walk_stackframe; } else { ... ksp = (unsigned long *)sp; while (!kstack_end(ksp)) { if (__kernel_text_address(pc) && unlikely(!fn(arg, pc))) break; pc = READ_ONCE_NOCHECK(*ksp++); } } Unlike the removed CONFIG_FRAME_POINTER version (which started at level = -1 to skip the placeholder pc), the !CONFIG_FRAME_POINTER loop calls fn(arg, pc) before reading *ksp++. As a result, arch_stack_walk() reports walk_stackframe before scanning its own stack frame, shifting stack_trace_save() and return_address() off by one and printing a bogus walk_stackframe entry in dump_backtrace(). Also, since all callers of walk_stackframe() are converted to arch_stack_walk() by this patch, walk_stackframe() has no remaining callers and its extern declaration in arch/riscv/include/asm/stacktrace.h is left without a definition when CONFIG_FRAME_POINTER=y. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110013750.36800-1-xueshuai@linux.alibaba.com?part=8 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (7 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 2:10 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 10/10] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue 2026-11-10 6:35 ` [PATCH v7 00/10] riscv: Add reliable stack unwinding for livepatch Karl Mehltretter 10 siblings, 1 reply; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> Now that the metadata frame records, the kunwind state machine and arch_stack_walk_reliable() are all in place, advertise the capability to the rest of the kernel: * select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so only the configurations with the tested metadata records and FP-based reliable walker enable it. * select HAVE_LIVEPATCH under the same condition and source kernel/livepatch/Kconfig so the livepatch menu is reachable from the RISC-V configuration. The 64BIT dependency is conservative scoping rather than a hard technical requirement: the metadata frame record, kunwind state machine and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack frame-record adjustment fixes a latent RV32 issue. However, the syscall livepatch selftest and module relocation path have only been exercised on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up once RV32 has equivalent coverage. This is split out from the unwinder change so the policy decision and the implementation can be reviewed and reverted independently. Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- arch/riscv/Kconfig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e7891a2a89da..4512f67bbfd9 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -186,6 +186,7 @@ config RISCV select HAVE_KRETPROBES # https://github.com/ClangBuiltLinux/linux/issues/1881 select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD + select HAVE_LIVEPATCH if FRAME_POINTER && 64BIT select HAVE_MOVE_PMD select HAVE_MOVE_PUD select HAVE_PAGE_SIZE_4KB @@ -195,6 +196,7 @@ config RISCV select HAVE_PERF_USER_STACK_DUMP select HAVE_POSIX_CPU_TIMERS_TASK_WORK select HAVE_REGS_AND_STACK_ACCESS_API + select HAVE_RELIABLE_STACKTRACE if FRAME_POINTER && 64BIT select HAVE_RETHOOK select HAVE_RSEQ select HAVE_RUST if RUSTC_SUPPORTS_RISCV && CC_IS_CLANG @@ -1415,3 +1417,5 @@ endmenu # "CPU Power Management" source "arch/riscv/kvm/Kconfig" source "drivers/acpi/Kconfig" + +source "kernel/livepatch/Kconfig" -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH 2026-11-10 1:37 ` [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue @ 2026-11-10 2:10 ` sashiko-bot 0 siblings, 0 replies; 39+ messages in thread From: sashiko-bot @ 2026-11-10 2:10 UTC (permalink / raw) To: Shuai Xue, palmer, pjw, aou Cc: alex, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, jkchen, linux-trace-kernel, linux-perf-users, live-patching Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [High] riscv: module: fault and UAF in apply_relocate_add() for klp relocs - [Medium] riscv: stacktrace: ftrace_caller() omits frames in reliable unwind - [Medium] riscv: stacktrace: uninitialized frame read in return_to_handler() - [Medium] riscv: stacktrace: arch_rethook_trampoline() omits caller frame -- commit 57947c1aaf895b44b13bb74dddecd8c57f82bb66 Author: Wang Han <wanghan@linux.alibaba.com> riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH This commit selects HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH on 64-bit RISC-V configurations with FRAME_POINTER enabled, and sources kernel/livepatch/Kconfig. > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index e7891a2a89da6..4512f67bbfd98 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -186,6 +186,7 @@ config RISCV > select HAVE_KRETPROBES > # https://github.com/ClangBuiltLinux/linux/issues/1881 > select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD > + select HAVE_LIVEPATCH if FRAME_POINTER && 64BIT [Severity: High] Can klp_apply_object_relocs() -> apply_relocate_add() trigger a store page fault and use-after-free when applying late .klp.rela.<module>.* relocations? During load_module(), complete_formation() marks module text and rodata (including .plt, .got, and .got.plt) read-only under CONFIG_STRICT_MODULE_RWX before klp_enable_patch() or klp_module_coming() runs. When apply_relocate_add() is later called on a module with mod->state != MODULE_STATE_UNFORMED, the relocation handlers in arch/riscv/kernel/module.c (such as riscv_insn_rmw()) and module_emit_got_entry() / module_emit_plt_entry() in arch/riscv/kernel/module-sections.c write directly to read-only module memory instead of using text patching helpers and flushing the instruction cache: arch/riscv/kernel/module.c:riscv_insn_rmw() { ... parcel[0] = cpu_to_le16(insn); parcel[1] = cpu_to_le16(insn >> 16); return 0; } In addition, module_frob_arch_sections() caches pointers into the temporary info->sechdrs buffer in mod->arch.plt.shdr, mod->arch.got.shdr, and mod->arch.got_plt.shdr, which free_copy() frees via vfree() at the end of load_module(): arch/riscv/kernel/module-sections.c:module_frob_arch_sections() { ... for (i = 0; i < ehdr->e_shnum; i++) { if (!strcmp(secstrings + sechdrs[i].sh_name, ".plt")) mod->arch.plt.shdr = sechdrs + i; else if (!strcmp(secstrings + sechdrs[i].sh_name, ".got")) mod->arch.got.shdr = sechdrs + i; else if (!strcmp(secstrings + sechdrs[i].sh_name, ".got.plt")) mod->arch.got_plt.shdr = sechdrs + i; } ... } When late livepatch relocations emit GOT or PLT entries, module_emit_got_entry() and module_emit_plt_entry() dereference these freed shdr pointers (e.g. got_sec->shdr->sh_addr), and re-applying relocations when a target module is reloaded increments num_entries past max_entries and hits BUG_ON(). > select HAVE_MOVE_PMD > select HAVE_MOVE_PUD > select HAVE_PAGE_SIZE_4KB > @@ -195,6 +196,7 @@ config RISCV > select HAVE_PERF_USER_STACK_DUMP > select HAVE_POSIX_CPU_TIMERS_TASK_WORK > select HAVE_REGS_AND_STACK_ACCESS_API > + select HAVE_RELIABLE_STACKTRACE if FRAME_POINTER && 64BIT [Severity: Medium] Does arch_stack_walk_reliable() silently omit the traced function and its caller when unwinding a task inside an ftrace callback or direct trampoline? With CONFIG_DYNAMIC_FTRACE_WITH_ARGS, the patchable entry of a traced function jumps to ftrace_caller before the function sets up its stack frame. At that point, t0 holds the traced function's PC, ra holds the return address in its caller, and s0 still holds the caller's frame pointer. In arch/riscv/kernel/mcount-dyn.S, SAVE_ABI_REGS saves t0, x1, and x8 into struct __arch_ftrace_regs without constructing frame records on the stack or updating s0: arch/riscv/kernel/mcount-dyn.S:SAVE_ABI_REGS { addi sp, sp, -FREGS_SIZE_ON_STACK REG_S t0, FREGS_EPC(sp) REG_S x1, FREGS_RA(sp) REG_S x8, FREGS_S0(sp) ... } When ftrace_caller calls op->func, op->func saves the unchanged s0 in its frame record. Unwinding that frame record in kunwind_next_frame_record() advances directly to the caller's caller with state->source == KUNWIND_SOURCE_FRAME, which arch_reliable_kunwind_consume_entry() accepts as reliable. During a livepatch transition, klp_check_stack() can miss that a preempted or blocked task is executing inside a patched function or its caller. [Severity: Medium] Can kunwind_next_frame_record() read uninitialized stack memory when unwinding through return_to_handler()? In arch/riscv/kernel/mcount.S, SAVE_RET_ABI_STATE allocates FREGS_SIZE_ON_STACK bytes, saves ra and s0 at low offsets from sp, and sets s0 to sp + FREGS_SIZE_ON_STACK before return_to_handler calls ftrace_return_to_handler(): arch/riscv/kernel/mcount.S:SAVE_RET_ABI_STATE { addi sp, sp, -FREGS_SIZE_ON_STACK REG_S ra, FREGS_RA(sp) REG_S s0, FREGS_S0(sp) REG_S a0, FREGS_A0(sp) REG_S a1, FREGS_A1(sp) addi s0, sp, FREGS_SIZE_ON_STACK } The 16 bytes at s0 - 16 (sp + FREGS_SIZE_ON_STACK - 16) are never initialized. When unwinding a task inside ftrace_return_to_handler(), kunwind_next_frame_record() restores fp = sp + FREGS_SIZE_ON_STACK and on the next step reads fp - sizeof(*record) as a struct frame_record, either triggering WARN_ON_ONCE(tsk == current) in kunwind_next_frame_record_meta() if those bytes happen to be zero or following a bogus frame pointer and skipping live frames while returning 0 (reliable). [Severity: Medium] Does unwinding from inside arch_rethook_trampoline_callback() omit the caller of the probed function while still reporting a reliable stack trace? When a kretprobed function returns to arch_rethook_trampoline(), its epilogue has already restored s0 to its caller's frame pointer, while the return address in the caller is held in current->rethooks. In arch/riscv/kernel/probes/rethook_trampoline.S, arch_rethook_trampoline allocates PT_SIZE_ON_STACK without initializing S_STACKFRAME (FRAME_META_TYPE_PT_REGS) or updating s0 before calling arch_rethook_trampoline_callback(): arch/riscv/kernel/probes/rethook_trampoline.S:arch_rethook_trampoline() { addi sp, sp, -(PT_SIZE_ON_STACK) save_all_base_regs move a0, sp /* pt_regs */ call arch_rethook_trampoline_callback ... } When kunwind unwinds arch_rethook_trampoline_callback()'s frame record, state->common.pc is inside arch_rethook_trampoline (past the entry address checked by is_kretprobe_trampoline()) and state->common.fp is the caller's frame pointer. The next unwind step reads the frame record of the caller's caller, skipping the caller of the probed function and returning 0 (reliable). -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110013750.36800-1-xueshuai@linux.alibaba.com?part=9 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v7 10/10] selftests/livepatch: Add RISC-V syscall wrapper prefix 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (8 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue @ 2026-11-10 1:37 ` Shuai Xue 2026-11-10 6:35 ` [PATCH v7 00/10] riscv: Add reliable stack unwinding for livepatch Karl Mehltretter 10 siblings, 0 replies; 39+ messages in thread From: Shuai Xue @ 2026-11-10 1:37 UTC (permalink / raw) To: palmer, pjw, aou Cc: alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, kmehltretter, oliver.yang, zhuo.song, xueshuai, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest From: Wang Han <wanghan@linux.alibaba.com> The syscall livepatch selftest resolves and patches a syscall wrapper symbol. To use that test for RISC-V livepatch validation, add the RISC-V FN_PREFIX definition for ARCH_HAS_SYSCALL_WRAPPER. Without this macro, the syscall livepatch selftest cannot resolve the RISC-V target symbol, and the syscall-related livepatch test fails on RISC-V. Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Reviewed-by: Martin Kaiser <martin@kaiser.cx> Tested-by: Martin Kaiser <martin@kaiser.cx> Signed-off-by: Wang Han <wanghan@linux.alibaba.com> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- .../testing/selftests/livepatch/test_modules/test_klp_syscall.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c index 08aacc0e14de..9baa2a5f84c9 100644 --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c @@ -24,6 +24,8 @@ #define FN_PREFIX __s390x_ #elif defined(__aarch64__) #define FN_PREFIX __arm64_ + #elif defined(__riscv) + #define FN_PREFIX __riscv_ #elif defined(__powerpc__) #define FN_PREFIX #else -- 2.39.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v7 00/10] riscv: Add reliable stack unwinding for livepatch 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue ` (9 preceding siblings ...) 2026-11-10 1:37 ` [PATCH v7 10/10] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue @ 2026-11-10 6:35 ` Karl Mehltretter 10 siblings, 0 replies; 39+ messages in thread From: Karl Mehltretter @ 2026-11-10 6:35 UTC (permalink / raw) To: Shuai Xue Cc: palmer, pjw, aou, alex, linux-riscv, oleg, rostedt, mhiramat, mark.rutland, peterz, mingo, acme, namhyung, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah, mpdesouza, martin, oliver.yang, zhuo.song, jkchen, linux-kernel, linux-trace-kernel, linux-perf-users, live-patching, linux-kselftest On Tue, Nov 10, 2026 at 09:37:40AM +0100, Shuai Xue wrote: > Changes since v6: > > * Rebase onto the current riscv/for-next (based on 7.4-rc1). > > * Fold in Karl Mehltretter's two patches [1] as 1/10 and 2/10. Patch > 1/10 stops arch_stack_walk() from reporting the walker's own frame as > the first entry, which corrupts the skip-frame accounting of > stack_trace_save() users (page_owner, perf), and adjusts > return_address() level accounting accordingly. Patch 2/10 adds a KUnit > test for the start frames. Both patches keep their original authorship; > I added my Reviewed-by/Tested-by and sign-off. I asked Karl on > 3 November whether he would like to see his patches folded into this > series; he has not replied so far. Since 8/10 reuses the walker > extraction from 1/10 for its !CONFIG_FRAME_POINTER fallback, his > patches are included here to keep the series self-contained. > Thank you for folding those patches in. I'm fine with that. Regards, Karl ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2026-11-10 6:35 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-14 9:26 [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Shuai Xue 2026-09-14 9:26 ` [PATCH v6 1/7] riscv: stacktrace: Add frame record metadata Shuai Xue 2026-09-14 9:40 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 2/7] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue 2026-09-14 9:32 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 3/7] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue 2026-09-14 9:32 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 4/7] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue 2026-09-14 9:33 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 5/7] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue 2026-09-14 9:39 ` sashiko-bot 2026-10-09 18:59 ` Karl Mehltretter 2026-10-10 3:38 ` Shuai Xue 2026-10-13 20:18 ` Martin Kaiser 2026-10-14 13:09 ` Shuai Xue 2026-09-14 9:26 ` [PATCH v6 6/7] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue 2026-09-14 9:38 ` sashiko-bot 2026-09-14 9:26 ` [PATCH v6 7/7] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue 2026-09-14 9:38 ` sashiko-bot 2026-09-29 7:28 ` [PATCH v6 0/7] riscv: Add reliable stack unwinding for livepatch Martin Kaiser 2026-10-08 11:42 ` Shuai Xue 2026-10-12 8:44 ` Martin Kaiser 2026-11-10 1:37 ` [PATCH v7 00/10] " Shuai Xue 2026-11-10 1:37 ` [PATCH v7 01/10] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk() Shuai Xue 2026-11-10 1:52 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 02/10] riscv: Add KUnit test for stack trace start frames Shuai Xue 2026-11-10 1:50 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 03/10] riscv: ftrace: use the frame pointer as the function graph return-address cookie Shuai Xue 2026-11-10 1:37 ` [PATCH v7 04/10] riscv: stacktrace: Add frame record metadata Shuai Xue 2026-11-10 1:54 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 05/10] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o Shuai Xue 2026-11-10 1:37 ` [PATCH v7 06/10] riscv: ftrace: always preserve s0 in dynamic ftrace register frame Shuai Xue 2026-11-10 1:37 ` [PATCH v7 07/10] riscv: stacktrace: introduce stack-bound tracking helpers Shuai Xue 2026-11-10 1:37 ` [PATCH v7 08/10] riscv: stacktrace: switch to frame-pointer based unwinder Shuai Xue 2026-11-10 2:20 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 09/10] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH Shuai Xue 2026-11-10 2:10 ` sashiko-bot 2026-11-10 1:37 ` [PATCH v7 10/10] selftests/livepatch: Add RISC-V syscall wrapper prefix Shuai Xue 2026-11-10 6:35 ` [PATCH v7 00/10] riscv: Add reliable stack unwinding for livepatch Karl Mehltretter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox