Live Patching
 help / color / mirror / Atom feed
* [PATCH 1/8] LoongArch: Add livepatch build (KLP) support
From: George Guo @ 2026-06-04  6:53 UTC (permalink / raw)
  To: chenhuacai, jpoimboe, peterz, jikos, mbenes, pmladek
  Cc: kernel, joe.lawrence, rostedt, ardb, nathan,
	nick.desaulniers+lkml, yangtiezhu, jiaxun.yang, xry111, liukexin,
	loongarch, live-patching, llvm, linux-kernel, George Guo
In-Reply-To: <20260604065317.219777-1-dongtai.guo@linux.dev>

From: George Guo <guodongtai@kylinos.cn>

This allows automated livepatch module generation using objtool.

Co-developed-by: Kexin Liu <liukexin@kylinos.cn>
Signed-off-by: Kexin Liu <liukexin@kylinos.cn>
Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 arch/loongarch/Kconfig | 1 +
 tools/objtool/Makefile | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 3b042dbb2c41..1dbf51ba9d6a 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -159,6 +159,7 @@ config LOONGARCH
 	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK
 	select HAVE_IRQ_TIME_ACCOUNTING
+	select HAVE_KLP_BUILD
 	select HAVE_KPROBES
 	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_KRETPROBES
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 94aabeee9736..83d645675ed9 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -8,7 +8,8 @@ ifeq ($(SRCARCH),x86)
 endif
 
 ifeq ($(SRCARCH),loongarch)
-	BUILD_ORC	   := y
+	BUILD_ORC    := y
+	ARCH_HAS_KLP := y
 endif
 
 ifeq ($(SRCARCH),arm64)
-- 
2.25.1


^ permalink raw reply related

* [PATCH 0/8] LoongArch: Add livepatch build (KLP) support
From: George Guo @ 2026-06-04  6:53 UTC (permalink / raw)
  To: chenhuacai, jpoimboe, peterz, jikos, mbenes, pmladek
  Cc: kernel, joe.lawrence, rostedt, ardb, nathan,
	nick.desaulniers+lkml, yangtiezhu, jiaxun.yang, xry111, liukexin,
	loongarch, live-patching, llvm, linux-kernel, George Guo

From: George Guo <guodongtai@kylinos.cn>

This series adds LoongArch support for the klp-build livepatch tooling,
enabling automated livepatch module generation using objtool on
LoongArch.

It is based on Josh Poimboeuf's klp-build series [1] (see base-commit
below) and extends it with the LoongArch-specific objtool and build
pieces, plus a few LoongArch toolchain/relocation fixes required to make
livepatch modules load and run correctly.

Overview:

 - Patch 1 wires up LoongArch klp-build (objtool-based livepatch module
   generation).
 - Patches 2-3 add the objtool bits LoongArch needs: arch_adjusted_addend()
   and the special section entry sizes (.altinstructions, __ex_table,
   __bug_table, __jump_table) so objtool can process livepatch modules.
 - Patches 4-5 fix kernel panics seen when loading livepatch modules,
   caused by LoongArch code-generation/relocation assumptions:
     * disable -mdirect-extern-access for klp builds (the module loader
       relies on GOT entries for external symbol references),
     * force -fPIC so same-compilation-unit symbol references relocate
       correctly.
 - Patch 6 fix EFI linking when building with -fdata-sections.
 - Patch 7 implements arch_jump_opcode_bytes() so klp checksums for
   jump/call instructions are position-independent, mirroring x86/arm64.
 - Patch 8 adds KLP_SYSCALL_DEFINEx() support for LoongArch.

Testing:
 - objtool builds cleanly for LoongArch.
 - Livepatch modules were generated and loaded on a LoongArch machine;
   the panics addressed by patches 4-5 no longer reproduce.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/log/?h=klp-build-arm64

George Guo (8):
  LoongArch: Add livepatch build (KLP) support
  objtool/LoongArch: Add arch_adjusted_addend() for KLP support
  LoongArch: Add special section entry sizes for KLP support
  livepatch/klp-build: disable direct-extern-access for LoongArch to fix
    kernel panic
  LoongArch: fix kernel panic with -fPIC for same-compilation-unit
    symbol references
  LoongArch: Fix EFI linking with -fdata-sections
  objtool/klp: Add LoongArch jump opcode bytes support
  klp-build: Add LoongArch syscall patching macro

 arch/loongarch/Kconfig                       |  1 +
 arch/loongarch/include/asm/alternative-asm.h |  5 ++-
 arch/loongarch/include/asm/alternative.h     |  6 ++-
 arch/loongarch/include/asm/asm-extable.h     | 10 ++++-
 arch/loongarch/include/asm/bug.h             | 15 ++++++-
 arch/loongarch/include/asm/jump_label.h      |  9 +++-
 arch/loongarch/kernel/asm-offsets.c          | 36 +++++++++++++++
 arch/loongarch/kernel/head.S                 |  1 +
 arch/loongarch/kernel/vmlinux.lds.S          |  2 +-
 include/linux/livepatch_helpers.h            | 22 ++++++++++
 scripts/livepatch/klp-build                  | 16 ++++++-
 tools/objtool/Makefile                       |  3 +-
 tools/objtool/arch/loongarch/decode.c        | 46 ++++++++++++++++++++
 13 files changed, 161 insertions(+), 11 deletions(-)


base-commit: 85afaba140a4b9f20fe8c8a64b24fc85f022d981
-- 
2.25.1


^ permalink raw reply

* Re: [PATCH] selftests: livepatch: set LC_ALL=C to fix locale-dependent test failure
From: Petr Mladek @ 2026-06-03 13:12 UTC (permalink / raw)
  To: Qiang Ma
  Cc: jpoimboe, jikos, mbenes, joe.lawrence, shuah, live-patching,
	linux-kselftest, linux-kernel
In-Reply-To: <20260527095929.1504032-1-maqianga@uniontech.com>

On Wed 2026-05-27 17:59:29, Qiang Ma wrote:
> When executing the command
> "make -C tools/testing/selftests TARGETS=livepatch run_tests",
> the following error message was reported.
> 
> TEST: livepatch interaction with ftrace_enabled sysctl ... not ok
> ...
> livepatch: sysctlo
> : setting key "kernel.ftrace_enabled": Device or resource busy
> livepatch: sysctl: setting key "kernel.ftrace_enabled": 设备或资源忙
> ...
> ERROR: livepatch kselftest(s) failed
> not ok 5 selftests: livepatch: test-ftrace.sh # exit=1
> 
> To fix it, set LC_ALL=C.
> 
> Signed-off-by: Qiang Ma <maqianga@uniontech.com>

JFYI, the patch has been committed into livepatching.git,
branch for-7.2-selftests.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH] selftests/livepatch: fix resource leak in test_klp_syscall init error path
From: Miroslav Benes @ 2026-06-03 13:09 UTC (permalink / raw)
  To: Rui Qi
  Cc: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah,
	live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260602124509.365996-1-qirui.001@bytedance.com>

On Tue, 02 Jun 2026 20:45:09 +0800, Rui Qi <qirui.001@bytedance.com> wrote:
> 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 0630ffd9d9a1..d631acae48b9 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> @@ -109,7 +109,12 @@ static int livepatch_init(void)
>  	 */
>  	npids = npids_pending;
>  
> -	return klp_enable_patch(&patch);
> +	ret = klp_enable_patch(&patch);
> +	if (ret) {
> +		sysfs_remove_file(klp_kobj, &klp_attr.attr);
> +		kobject_put(klp_kobj);
> +	}
> +	return ret;

Is sysfs_remove_file() needed? I think that kobject_put() should remove
it automatically since the object is bound to sysfs.

-- 
Miroslav


^ permalink raw reply

* Re: [PATCH 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Chen Pei @ 2026-06-03  7:14 UTC (permalink / raw)
  To: wanghan
  Cc: acme, alex, andybnac, aou, bjorn, catalin.marinas, conor.dooley,
	cp0613, debug, jikos, joe.lawrence, jpoimboe, linux-kernel,
	linux-kselftest, linux-perf-users, linux-riscv,
	linux-trace-kernel, live-patching, mark.rutland, mbenes, mhiramat,
	mingo, namhyung, palmer, peterz, pjw, pmladek, puranjay, rostedt,
	shuah
In-Reply-To: <20260527123530.2593918-2-wanghan@linux.alibaba.com>

On Wed, 27 May 2026 20:35:23 +0800, wanghan@linux.alibaba.com wrote:

> On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
> and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
> while ftrace reports zero usable entries and the early selftests fail:
> 
>   [    0.000000] ftrace section at ffffffff8101da98 sorted properly
>   [    0.000000] ftrace: allocating 0 entries in 128 pages
>   [    0.054999] Testing tracer function: .. no entries found ..FAILED!
>   [    0.172407] tracer: function failed selftest, disabling
>   [    0.178186] Failed to init function_graph tracer, init returned -19
> 
> Handle RISC-V like arm64 for the function-range check and allow
> patchable entries up to 8 bytes before the function address.
> 
> With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
> the vmlinux ftrace table is populated and dynamic ftrace still works:
> 
>   [    0.000000] ftrace: allocating 46749 entries in 184 pages
>   [    0.051115] Testing tracer function: PASSED
>   [    1.283782] Testing dynamic ftrace: PASSED
>   [    6.275456] Testing tracer function_graph: PASSED
> 
> Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")

Oops, sorry for missing that. Thanks for the quick fix!

Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>

^ permalink raw reply

* Re: [PATCH v2 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Shuai Xue @ 2026-06-03  2:10 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
	Chen Pei, Andy Chiu, Björn Töpel, Deepak Gupta,
	Puranjay Mohan, Conor Dooley, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Petr Mladek, Joe Lawrence, Shuah Khan,
	Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260601095746.70c01d24@fedora>



On 6/1/26 9:57 PM, Steven Rostedt wrote:
> On Mon, 1 Jun 2026 14:17:08 +0800
> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> 
>>> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
>>> index e8ed11c680c6..4c10e85bb5af 100644
>>> --- a/scripts/sorttable.c
>>> +++ b/scripts/sorttable.c
>>> @@ -891,17 +891,21 @@ static int do_file(char const *const fname, void *addr)
>>>    	table_sort_t custom_sort = NULL;
>>>    
>>>    	switch (elf_map_machine(ehdr)) {
>>> -	case EM_AARCH64:
>>>    #ifdef MCOUNT_SORT_ENABLED
>>> +	case EM_AARCH64:
>>>    		sort_reloc = true;
>>>    		rela_type = 0x403;
>>> -		/* arm64 uses patchable function entry placing before function */
>>> +		/* fallthrough */
>>> +	case EM_RISCV:
>>> +		/* arm64 and RISC-V place patchable entries before the function */
>>>    		before_func = 8;
>>
>> Nit: The shared comment now sits under `case EM_RISCV:` but the two
>> lines above it (sort_reloc / rela_type = 0x403) are strictly
>> arm64-only — they configure the RELA-based weak-function fixup that
>> RISC-V does not need. On a quick read it is easy to wonder if RISC-V
>> is implicitly inheriting that path. Splitting the comments would
>> help, e.g.:
>>
>>          case EM_AARCH64:
>>              /* arm64 needs RELA-based weak-function fixup */
>>              sort_reloc = true;
>>              rela_type = 0x403;
>>              /* fallthrough */
>>          case EM_RISCV:
>>              /* arm64 and RISC-V place patchable entries before the function */
>>              before_func = 8;
> 
> Makes sense.
> 
> Care to send a v3?
> 
> -- Steve

Hi, Steve,

It's a pure comment cosmetic, not worth a respin on its own. But for the
rest of the feedback on this series (the frame-record metadata contract
in patch 2 and the dead state->regs field / Call Trace output change in
patch 6) are the ones actually worth a new version.

Just to get the routing straight: are you planning to pick this one up
through the tracing tree on its own?

It feels like a good candidate for that -- it's an independent
regression fix (Fixes: 0ca1724b56af) that breaks *all* RISC-V dynamic
ftrace, not just livepatch, so it shouldn't have to wait on the rest of
the livepatch series.

Thanks.
Shuai



^ permalink raw reply

* Re: [PATCH v2 8/8] selftests/livepatch: Add RISC-V syscall wrapper prefix
From: Shuai Xue @ 2026-06-03  1:54 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-9-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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.
> 
> Signed-off-by: Wang Han <wanghan@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 dd802783ea84..275e4b10cf59 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> @@ -18,6 +18,8 @@
>   #define FN_PREFIX __s390x_
>   #elif defined(__aarch64__)
>   #define FN_PREFIX __arm64_
> +#elif defined(__riscv)
> +#define FN_PREFIX __riscv_
>   #else
>   /* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
>   #define FN_PREFIX

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 7/8] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
From: Shuai Xue @ 2026-06-03  1:49 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-8-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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 that actually have the metadata records
>      and the FP-based reliable walker enable it.


The 64BIT gate is conservative scoping rather than a hard technical
requirement: the metadata frame record, kunwind state machine and
arch_stack_walk_reliable() all build on RV32 too (and the
call_on_irq_stack change in patch 2/8 actually fixes a latent RV32
issue). However, the syscall livepatch selftest and module relocation
path have only been exercised on RV64 (QEMU virt SMP=2/4/8). The
64BIT gate can be dropped in a follow-up once RV32 has equivalent
coverage.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 6/8] riscv: stacktrace: switch to frame-pointer based unwinder
From: Shuai Xue @ 2026-06-03  1:35 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-7-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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.
> 
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
>   arch/riscv/kernel/ftrace.c         |   6 +-
>   arch/riscv/kernel/perf_callchain.c |   2 +-
>   arch/riscv/kernel/stacktrace.c     | 560 ++++++++++++++++++++++++-----
>   3 files changed, 472 insertions(+), 96 deletions(-)
> 
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index b430edfb83f4..5d55199a9230 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -242,7 +242,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;
>   }
>   
> @@ -264,7 +265,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 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 2692d3a06afa..0d76320b3a29 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,468 @@ 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->regs = NULL;

state->regs is a dead field, and kunwind_next_regs_pc() clears
         it in a way that contradicts both arm64 and your own
         init_from_regs.

struct kunwind_state has a `struct pt_regs *regs`, but I can't find any
reader of it anywhere in the file — arch_kunwind_consume_entry() and
arch_reliable_kunwind_consume_entry() only ever read common.pc and
source. It is written in three places:

     kunwind_init():           state->regs = NULL;
     kunwind_init_from_regs():  state->regs = regs;     /* not cleared */
     kunwind_next_regs_pc():    state->regs = regs;
                                state->common.pc = regs->epc;
                                state->common.fp = frame_pointer(regs);
                                state->regs = NULL;      /* cleared! */

Two things:

   (a) The field has no consumer, so it's currently dead.

   (b) In kunwind_next_regs_pc() you set state->regs = regs and then
       immediately reset it to NULL two lines later. The arm64 reference
       does *not* clear it there, and your own kunwind_init_from_regs()
       leaves it set. So the three REGS_PC producers disagree on whether
       ->regs is valid.

It's harmless today only because nothing reads ->regs. But the moment
someone adds a consumer (e.g. to expose the pt_regs at an exception
boundary for a reliable dump), the stray `state->regs = NULL;` in
kunwind_next_regs_pc() becomes a silent bug.

Please either:
   - drop the field and all three writes, if it's genuinely unused, or
   - keep it and remove the `state->regs = NULL;` in
     kunwind_next_regs_pc() so it matches arm64 and init_from_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),
> +		STACKINFO_CPU(tsk, irq),
> +#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 the LR was live when the exception was taken, and

Nit: s/LR/ra/ here. RISC-V has no link register; the equivalent is the
return-address register ra (x1). You already localized this correctly in
the kr_cur docstring ("replacement ra value"), so this comment is just an
oversight.

Thanks.
Shuai


^ permalink raw reply

* Re: [PATCH v2 5/8] riscv: stacktrace: introduce stack-bound tracking helpers
From: Shuai Xue @ 2026-06-03  1:23 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-6-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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.
> 
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
>   arch/riscv/include/asm/stacktrace.h        |  65 ++++++++-
>   arch/riscv/include/asm/stacktrace/common.h | 159 +++++++++++++++++++++
>   2 files changed, 222 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..bc87c4940379 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,70 @@ 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
> + */
> +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);
> +}
> +
> +/*
> + * 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..87d6d40672f3
> --- /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) 2024

Nit: The new common.h carries "Copyright (C) 2024", but this is a 2026
submission.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai


^ permalink raw reply

* [PATCH] selftests/livepatch: fix resource leak in test_klp_syscall init error path
From: Rui Qi @ 2026-06-02 12:45 UTC (permalink / raw)
  To: jpoimboe, jikos, mbenes, pmladek
  Cc: joe.lawrence, shuah, live-patching, linux-kselftest, linux-kernel,
	Rui Qi

In livepatch_init(), if klp_enable_patch() fails, the previously
created kobject and sysfs file are never cleaned up, causing a
resource leak. Capture the return value and add proper cleanup
on the error path.

Signed-off-by: Rui Qi <qirui.001@bytedance.com>
---
 .../selftests/livepatch/test_modules/test_klp_syscall.c    | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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 0630ffd9d9a1..d631acae48b9 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
@@ -109,7 +109,12 @@ static int livepatch_init(void)
 	 */
 	npids = npids_pending;
 
-	return klp_enable_patch(&patch);
+	ret = klp_enable_patch(&patch);
+	if (ret) {
+		sysfs_remove_file(klp_kobj, &klp_attr.attr);
+		kobject_put(klp_kobj);
+	}
+	return ret;
 }
 
 static void livepatch_exit(void)
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH v2 4/8] riscv: ftrace: always preserve s0 in dynamic ftrace register frame
From: Shuai Xue @ 2026-06-02 11:37 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-5-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> The dynamic ftrace entry/exit only saved s0 (the architectural frame
> pointer) when HAVE_FUNCTION_GRAPH_FP_TEST was selected. The upcoming
> reliable frame-pointer unwinder needs s0 to be present in
> ftrace_regs unconditionally so it can use the frame pointer as the
> function-graph return-address cookie regardless of FP_TEST.

Nit: A prefered commit log:

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.

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>

Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 3/8] riscv: stacktrace: disable KASAN instrumentation for stacktrace.o
From: Shuai Xue @ 2026-06-02 11:22 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-4-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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.
> 
> Mark stacktrace.o as not KASAN-instrumented, matching the arm, arm64
> and x86 treatment of their stack unwinding code. This is a prerequisite
> preference for the upcoming reliable unwinder, but the change is valid
> on its own.
> 
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
>   arch/riscv/kernel/Makefile | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index cabb99cadfb6..1cb6c9ab2981 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -44,6 +44,11 @@ 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
> +

I checked the three referenced arches:
   - arm    (arch/arm/kernel/Makefile):    KASAN only
   - arm64  (arch/arm64/kernel/Makefile):  KASAN only
   - x86    (arch/x86/kernel/Makefile):    KASAN *and* KCOV
            (KCOV_INSTRUMENT_stacktrace.o := n, plus dumpstack and
             the unwind_*.o TUs)

So as written, this patch matches arm/arm64 but NOT x86. KCOV
instruments every basic-block edge; the unwinder is a hot path
(doubly so under KASAN, where it runs on every alloc/free), so the
same rationale that justifies disabling KASAN applies to KCOV. I'd
suggest making the claim true by adding:

  KCOV_INSTRUMENT_stacktrace.o := n

(RISC-V keeps its entire unwinder in stacktrace.o, so unlike x86
there's no dumpstack/unwind_*.o to also annotate — the single TU
covers the equivalent scope.)

Alternatively, if you'd rather keep it minimal, just drop "and x86"
from the changelog so the claim matches the code.


Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 2/8] riscv: stacktrace: Add frame record metadata
From: Shuai Xue @ 2026-06-02 11:18 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-3-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> 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 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;
>    * _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;
>    * call_on_irq_stack still reserves an aligned stack slot, but links the
>      saved {fp, ra} with the raw frame-record size so s0 points at the
>      RISC-V frame record rather than past the alignment padding.
> 
> These changes keep s0 reserved for the frame-pointer chain at task and
> stack-switch boundaries.
> 
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
>   arch/riscv/include/asm/ptrace.h           |  9 ++++
>   arch/riscv/include/asm/stacktrace/frame.h | 53 +++++++++++++++++++++++
>   arch/riscv/kernel/asm-offsets.c           |  4 ++
>   arch/riscv/kernel/entry.S                 | 30 +++++++++++--
>   arch/riscv/kernel/head.S                  | 23 ++++++++++
>   arch/riscv/kernel/process.c               | 31 ++++++++++++-
>   6 files changed, 144 insertions(+), 6 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 af827448a609..8dfcb5a44bb8 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);
> @@ -501,6 +504,7 @@ void asm_offsets(void)
>   	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
>   
>   	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
> +	DEFINE(STACKFRAME_RECORD_SIZE, sizeof(struct stackframe));
>   	OFFSET(STACKFRAME_FP, stackframe, fp);
>   	OFFSET(STACKFRAME_RA, stackframe, ra);
>   #ifdef CONFIG_FUNCTION_TRACER
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index d011fb51c59a..9cae0e1eba1c 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>
> @@ -193,6 +194,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 + STACKFRAME_FP)(sp) /* stackframe.record.fp = 0 */
> +	REG_S zero, (S_STACKFRAME + STACKFRAME_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
> +

One spot for symmetry (non-blocking, robustness only):

handle_kernel_stack_overflow in entry.S allocates a full
PT_SIZE_ON_STACK frame (including the new stackframe metadata fields)
but, unlike .Lsave_context, never initialises stackframe.{record,type}
nor repoints s0 at the metadata. Those three words are therefore left
as whatever was on the overflow_stack.

In practice this is currently harmless: handle_bad_stack() only calls
__show_regs() (register dump, no unwind) followed by panic(), so the
reliable unwinder never actually consumes that metadata today. So this
is not an active bug — purely a robustness / symmetry gap.

It would still be worth initialising it, because the moment someone
adds a dump_stack() here, or another CPU NMI-backtraces this task, or a
kdump image is walked offline via the frame-record chain, the garbage
type byte would mislead the unwinder. Since the overflow path is by
definition entered from kernel context, FRAME_META_TYPE_PT_REGS is the
right type, and it has the nice property that the unwinder will resume
from frame_pointer(regs)==regs->s0 (the pre-overflow s0 is already
saved into PT_S0 by save_from_x6_to_x31), giving the pre-overflow call
chain instead of a hard stop.


>   	/*
>   	 * Set the scratch register to 0, so that if a recursive exception
>   	 * occurs, the exception vector knows it came from the kernel
> @@ -357,8 +379,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
> @@ -383,7 +405,7 @@ SYM_FUNC_START(call_on_irq_stack)
>   	addi	sp, sp, -STACKFRAME_SIZE_ON_STACK
>   	REG_S	ra, STACKFRAME_RA(sp)
>   	REG_S	s0, STACKFRAME_FP(sp)
> -	addi	s0, sp, STACKFRAME_SIZE_ON_STACK
> +	addi	s0, sp, STACKFRAME_RECORD_SIZE
>   
>   	/* Switch to the per-CPU shadow call stack */
>   	scs_save_current
> @@ -399,7 +421,7 @@ SYM_FUNC_START(call_on_irq_stack)
>   	scs_load_current
>   
>   	/* Switch back to the thread stack and restore ra and s0 */
> -	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
> +	addi	sp, s0, -STACKFRAME_RECORD_SIZE

Worth calling out explicitly that this is more than a cosmetic refactor:
on RV32 the previous code is actually wrong, and this hunk fixes it.

   STACKFRAME_SIZE_ON_STACK = ALIGN(sizeof(struct stackframe), STACK_ALIGN)
   STACKFRAME_RECORD_SIZE   = sizeof(struct stackframe)

   RV64: sizeof(stackframe) == STACK_ALIGN == 16, so the two are equal
         and the old code happened to work.
   RV32: sizeof(stackframe) == 8 but STACK_ALIGN == 16, so the old
         "addi s0, sp, STACKFRAME_SIZE_ON_STACK" left s0 pointing 8 bytes
         past the saved {fp, ra} pair, into the alignment padding. An FP
         unwinder that derives the frame record from s0 (e.g. via
         "(struct stackframe *)s0 - 1" or fixed -8/-16(s0) loads) would
         then read garbage instead of the saved fp/ra at the IRQ-stack

After the change s0 lands exactly at the end of the {fp, ra} record on
both RV32 and RV64, while the aligned slot is still reserved by the
unchanged "addi sp, sp, -STACKFRAME_SIZE_ON_STACK" / matching restore.

Could you mention this in the v3 commit message? It's load-bearing
context for anyone bisecting an RV32 unwind regression later, and it
also justifies why the change is correct to apply ahead of the reliable
unwinder rather than folded into it.

>   	REG_L	ra, STACKFRAME_RA(sp)
>   	REG_L	s0, STACKFRAME_FP(sp)
>   	addi	sp, sp, STACKFRAME_SIZE_ON_STACK
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index f6a8ca49e627..00e16a24f149 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 + STACKFRAME_FP)(sp)
> +	REG_S zero, (S_STACKFRAME + STACKFRAME_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 b2df7f72241a..5212926b926b 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,18 @@ 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. The type
> +		 * field is inherited from the parent's pt_regs.
> +		 */
> +		childregs->stackframe.record.fp = 0;
> +		childregs->stackframe.record.ra = 0;

This relies on the parent always entering kernel via handle_exception
on a user->kernel boundary (which writes FRAME_META_TYPE_FINAL).
That is true for fork()/clone() today, but:

   - The kernel-thread path right above explicitly assigns type =
     FINAL, so the user-thread path looks asymmetric and like a
     possible omission to anyone reading it cold.
   - A future caller invoking kernel_clone() from a nested-kernel
     context (parent pt_regs.type == PT_REGS) would silently produce
     a broken unwind boundary on the new task.

Recommend explicitly setting it here too:

       childregs->stackframe.type = FRAME_META_TYPE_FINAL;

Even if currently redundant, it is one assignment, costs nothing, is
self-documenting, and fails closed instead of open.


Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Steven Rostedt @ 2026-06-01 13:57 UTC (permalink / raw)
  To: Shuai Xue
  Cc: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
	Chen Pei, Andy Chiu, Björn Töpel, Deepak Gupta,
	Puranjay Mohan, Conor Dooley, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Petr Mladek, Joe Lawrence, Shuah Khan,
	Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <0a913398-3d0c-472e-89c4-062052eae04d@linux.alibaba.com>

On Mon, 1 Jun 2026 14:17:08 +0800
Shuai Xue <xueshuai@linux.alibaba.com> wrote:

> > diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> > index e8ed11c680c6..4c10e85bb5af 100644
> > --- a/scripts/sorttable.c
> > +++ b/scripts/sorttable.c
> > @@ -891,17 +891,21 @@ static int do_file(char const *const fname, void *addr)
> >   	table_sort_t custom_sort = NULL;
> >   
> >   	switch (elf_map_machine(ehdr)) {
> > -	case EM_AARCH64:
> >   #ifdef MCOUNT_SORT_ENABLED
> > +	case EM_AARCH64:
> >   		sort_reloc = true;
> >   		rela_type = 0x403;
> > -		/* arm64 uses patchable function entry placing before function */
> > +		/* fallthrough */
> > +	case EM_RISCV:
> > +		/* arm64 and RISC-V place patchable entries before the function */
> >   		before_func = 8;  
> 
> Nit: The shared comment now sits under `case EM_RISCV:` but the two
> lines above it (sort_reloc / rela_type = 0x403) are strictly
> arm64-only — they configure the RELA-based weak-function fixup that
> RISC-V does not need. On a quick read it is easy to wonder if RISC-V
> is implicitly inheriting that path. Splitting the comments would
> help, e.g.:
> 
>         case EM_AARCH64:
>             /* arm64 needs RELA-based weak-function fixup */
>             sort_reloc = true;
>             rela_type = 0x403;
>             /* fallthrough */
>         case EM_RISCV:
>             /* arm64 and RISC-V place patchable entries before the function */
>             before_func = 8;

Makes sense.

Care to send a v3?

-- Steve

^ permalink raw reply

* Re: [PATCH v2 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Shuai Xue @ 2026-06-01  6:17 UTC (permalink / raw)
  To: Wang Han, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
	Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
	Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
	Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, oliver.yang, zhuo.song, jkchen, linux-riscv,
	linux-kernel, linux-trace-kernel, live-patching, linux-kselftest,
	linux-perf-users
In-Reply-To: <20260528082310.1994388-2-wanghan@linux.alibaba.com>



On 5/28/26 4:23 PM, Wang Han wrote:
> RISC-V uses -fpatchable-function-entry=8,4 when the compressed ISA is
> enabled and -fpatchable-function-entry=4,2 otherwise. In both cases, the
> patchable NOP area starts 8 bytes before the function symbol address.
> The __mcount_loc entries therefore point at the patchable NOP area
> associated with a function, while nm reports the function symbol at the
> entry address used for the function range check.
> 
> After RISC-V selected HAVE_BUILDTIME_MCOUNT_SORT, sorttable started
> applying that range check at build time. Without allowing entries just
> before the reported function address, the mcount sorter treats valid
> RISC-V ftrace callsites as invalid weak-function entries and writes
> them back as zero. The resulting kernel boots with no ftrace entries,
> breaking dynamic ftrace and users such as livepatch.
> 
> The failure is silent during the final link because zeroing weak-function
> entries is an expected sorttable operation. At boot, those zero entries
> are skipped by ftrace_process_locs(), so the only obvious symptom is that
> the vmlinux ftrace table has lost valid callsites and ftrace users cannot
> attach to them.
> 
> CONFIG_FTRACE_SORT_STARTUP_TEST also reports the table as sorted in this
> state: it only checks that the __mcount_loc entries are in ascending
> order, which a fully zeroed table trivially satisfies. The original
> commit relied on this check and did not see the regression.
> 
> On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
> and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
> while ftrace reports zero usable entries and the early selftests fail:
> 
>    [    0.000000] ftrace section at ffffffff8101da98 sorted properly
>    [    0.000000] ftrace: allocating 0 entries in 128 pages
>    [    0.054999] Testing tracer function: .. no entries found ..FAILED!
>    [    0.172407] tracer: function failed selftest, disabling
>    [    0.178186] Failed to init function_graph tracer, init returned -19
> 
> Handle RISC-V like arm64 for the function-range check and allow
> patchable entries up to 8 bytes before the function address.
> 
> With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
> the vmlinux ftrace table is populated and dynamic ftrace still works:
> 
>    [    0.000000] ftrace: allocating 46749 entries in 184 pages
>    [    0.051115] Testing tracer function: PASSED
>    [    1.283782] Testing dynamic ftrace: PASSED
>    [    6.275456] Testing tracer function_graph: PASSED
> 
> Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
> Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
>   scripts/sorttable.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index e8ed11c680c6..4c10e85bb5af 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -891,17 +891,21 @@ static int do_file(char const *const fname, void *addr)
>   	table_sort_t custom_sort = NULL;
>   
>   	switch (elf_map_machine(ehdr)) {
> -	case EM_AARCH64:
>   #ifdef MCOUNT_SORT_ENABLED
> +	case EM_AARCH64:
>   		sort_reloc = true;
>   		rela_type = 0x403;
> -		/* arm64 uses patchable function entry placing before function */
> +		/* fallthrough */
> +	case EM_RISCV:
> +		/* arm64 and RISC-V place patchable entries before the function */
>   		before_func = 8;

Nit: The shared comment now sits under `case EM_RISCV:` but the two
lines above it (sort_reloc / rela_type = 0x403) are strictly
arm64-only — they configure the RELA-based weak-function fixup that
RISC-V does not need. On a quick read it is easy to wonder if RISC-V
is implicitly inheriting that path. Splitting the comments would
help, e.g.:

        case EM_AARCH64:
            /* arm64 needs RELA-based weak-function fixup */
            sort_reloc = true;
            rela_type = 0x403;
            /* fallthrough */
        case EM_RISCV:
            /* arm64 and RISC-V place patchable entries before the function */
            before_func = 8;


> +#else
> +	case EM_AARCH64:
> +	case EM_RISCV:
>   #endif
>   		/* fallthrough */
>   	case EM_386:
>   	case EM_LOONGARCH:
> -	case EM_RISCV:
>   	case EM_S390:
>   	case EM_X86_64:
>   		custom_sort = sort_relative_table_with_data;

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

^ permalink raw reply

* Re: [PATCH v2 3/4] livepatch: deprecate stack_order
From: Yafang Shao @ 2026-05-29  9:13 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: song, jpoimboe, joe.lawrence, jikos, mbenes, pmladek,
	live-patching
In-Reply-To: <20260529045446.B22791F00893@smtp.kernel.org>

On Fri, May 29, 2026 at 12:54 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The commit entirely removes the `stack_order` sysfs attribute while only marking it as `<deprecated>` in the ABI documentation, causing immediate backwards compatibility breakage and contradicting documentation guidelines.
> --
>
> commit 041b2dbba1a7397626104dd773fc1f37228d382c
> Author: Yafang Shao <laoar.shao@gmail.com>
>
> livepatch: deprecate stack_order
>
> This commit deprecates the stack_order sysfs attribute for atomic-replace
> livepatches, since a single function can only be modified by a unique
> replace_set.
>
> > diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
> > index 6d75235a6a2e4..fddad93dba797 100644
> > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> > @@ -62,6 +62,7 @@ Description:
> >               are applied to the system. If multiple live patches modify the same
> >               function, the implementation with the biggest 'stack_order' number
> >               is used, unless a transition is currently in progress.
> > +             <deprecated>
>
> [ ... ]
>
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 969fea2a92631..6d65f839f4420 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> [ ... ]
> > @@ -457,38 +456,15 @@ static ssize_t replace_set_show(struct kobject *kobj,
> >       return sysfs_emit(buf, "%u\n", patch->replace_set);
> >  }
> >
> > -static ssize_t stack_order_show(struct kobject *kobj,
> > -                             struct kobj_attribute *attr, char *buf)
> > -{
> > -     struct klp_patch *patch, *this_patch;
> > -     int stack_order = 0;
> > -
> > -     this_patch = container_of(kobj, struct klp_patch, kobj);
> > -
> > -     mutex_lock(&klp_mutex);
> > -
> > -     klp_for_each_patch(patch) {
> > -             stack_order++;
> > -             if (patch == this_patch)
> > -                     break;
> > -     }
> > -
> > -     mutex_unlock(&klp_mutex);
> > -
> > -     return sysfs_emit(buf, "%d\n", stack_order);
> > -}
> > -
> >  static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
> >  static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
> >  static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
> >  static struct kobj_attribute replace_set_kobj_attr = __ATTR_RO(replace_set);
> > -static struct kobj_attribute stack_order_kobj_attr = __ATTR_RO(stack_order);
> >  static struct attribute *klp_patch_attrs[] = {
> >       &enabled_kobj_attr.attr,
> >       &transition_kobj_attr.attr,
> >       &force_kobj_attr.attr,
> >       &replace_set_kobj_attr.attr,
> > -     &stack_order_kobj_attr.attr,
> >       NULL
> >  };
>
> [Severity: Critical]
> The commit message and documentation state that the stack_order attribute
> is being deprecated, but this code completely removes the attribute. Does
> this abruptly break ABI compatibility for userspace tools that expect this
> sysfs file to exist?
>
> Normally, deprecating an interface requires keeping it around for a
> transition period (perhaps returning a dummy value) rather than deleting
> it immediately. Also, if it were actually removed, shouldn't the ABI
> documentation be moved to the removed/ directory instead of just appending
> <deprecated> to the existing entry?

I will wait for the feedback from the maintainers.

-- 
Regards
Yafang

^ permalink raw reply

* Re: [PATCH v2 4/4] selftests/livepatch: Update tests for replace_set
From: Yafang Shao @ 2026-05-29  9:12 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: jikos, mbenes, song, joe.lawrence, pmladek, jpoimboe,
	live-patching
In-Reply-To: <20260529051659.BEA391F00893@smtp.kernel.org>

On Fri, May 29, 2026 at 1:17 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The 'atomic replace livepatch' test case in test-livepatch.sh was completely removed instead of being updated, resulting in lost test coverage and leaving behind dead variables.
> --
>
> commit 99bb60c2ba8319ec21289c482f1ea304e282396c
> Author: Yafang Shao <laoar.shao@gmail.com>
>
> selftests/livepatch: Update tests for replace_set
>
> This commit updates the livepatch selftests to use "replace_set" instead
> of "replace". It configures different livepatches to share or have different
> replace_sets to test atomic and non-atomic replace behaviors.
>
> > diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
> > index c44c5341a2f1e..042999856267f 100755
> > --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> > +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> [ ... ]
> > @@ -108,96 +108,6 @@ livepatch: '$MOD_LIVEPATCH1': completing unpatching transition
> >  livepatch: '$MOD_LIVEPATCH1': unpatching complete
> >  % rmmod $MOD_LIVEPATCH1"
> >
> > -
> > -# - load a livepatch that modifies the output from /proc/cmdline and
> > -#   verify correct behavior
> > -# - load two additional livepatches and check the number of livepatch modules
> > -#   applied
> > -# - load an atomic replace livepatch and check that the other three modules were
> > -#   disabled
> > -# - remove all livepatches besides the atomic replace one and verify that the
> > -#   atomic replace livepatch is still active
> > -# - remove the atomic replace livepatch and verify that none are active
> > -
> > -start_test "atomic replace livepatch"
> > -
> > -load_lp $MOD_LIVEPATCH1
>
> [Severity: Medium]
> Was this test case intentionally removed completely instead of being updated for
> the new replace_set semantics?
>
> The commit message does not mention removing this test. This removal causes a
> loss of test coverage that previously verified an atomic replace successfully
> disables multiple existing livepatches.

This test case is invalid for the replace_set, since only one
livepatch will be replaced.

>
> Additionally, removing this test leaves the script variables MOD_LIVEPATCH2
> and MOD_LIVEPATCH3 unused in test-livepatch.sh. Could this test be adapted
> to use replace_set instead?

I will update it.

-- 
Regards
Yafang

^ permalink raw reply

* Re: [PATCH v2 2/4] livepatch: Support scoped atomic replace using replace_set
From: Yafang Shao @ 2026-05-29  9:11 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: jikos, pmladek, song, mbenes, jpoimboe, joe.lawrence,
	live-patching
In-Reply-To: <20260529043212.1F2ED1F00893@smtp.kernel.org>

On Fri, May 29, 2026 at 12:32 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] A malformed livepatch module with a missing function `old_name` will cause a kernel panic during patch compatibility checking.

The same issue with Patch #1.

-- 
Regards
Yafang

^ permalink raw reply

* Re: [PATCH v2 1/4] livepatch: Make klp_find_func() non-static
From: Yafang Shao @ 2026-05-29  9:07 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: jikos, joe.lawrence, song, mbenes, jpoimboe, pmladek,
	live-patching
In-Reply-To: <20260529040130.95A9C1F00893@smtp.kernel.org>

On Fri, May 29, 2026 at 12:01 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Low] klp_find_func() is improperly exposed in the global public header, breaking encapsulation and introducing potential build/linkage errors.

I will update it.

>
> Pre-existing issues:
> - [High] klp_find_func() lacks input validation, exacerbating a pre-existing NULL pointer dereference vulnerability during livepatch loading.

Valid. will fix this issue.

-- 
Regards
Yafang

^ permalink raw reply

* Re: [PATCH v2 4/4] selftests/livepatch: Update tests for replace_set
From: sashiko-bot @ 2026-05-29  5:16 UTC (permalink / raw)
  To: jikos, mbenes, song, joe.lawrence, pmladek, jpoimboe
  Cc: Yafang Shao, live-patching
In-Reply-To: <20260529034542.68766-5-laoar.shao@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The 'atomic replace livepatch' test case in test-livepatch.sh was completely removed instead of being updated, resulting in lost test coverage and leaving behind dead variables.
--

commit 99bb60c2ba8319ec21289c482f1ea304e282396c
Author: Yafang Shao <laoar.shao@gmail.com>

selftests/livepatch: Update tests for replace_set

This commit updates the livepatch selftests to use "replace_set" instead
of "replace". It configures different livepatches to share or have different
replace_sets to test atomic and non-atomic replace behaviors.

> diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
> index c44c5341a2f1e..042999856267f 100755
> --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
[ ... ]
> @@ -108,96 +108,6 @@ livepatch: '$MOD_LIVEPATCH1': completing unpatching transition
>  livepatch: '$MOD_LIVEPATCH1': unpatching complete
>  % rmmod $MOD_LIVEPATCH1"
>  
> -
> -# - load a livepatch that modifies the output from /proc/cmdline and
> -#   verify correct behavior
> -# - load two additional livepatches and check the number of livepatch modules
> -#   applied
> -# - load an atomic replace livepatch and check that the other three modules were
> -#   disabled
> -# - remove all livepatches besides the atomic replace one and verify that the
> -#   atomic replace livepatch is still active
> -# - remove the atomic replace livepatch and verify that none are active
> -
> -start_test "atomic replace livepatch"
> -
> -load_lp $MOD_LIVEPATCH1

[Severity: Medium]
Was this test case intentionally removed completely instead of being updated for
the new replace_set semantics?

The commit message does not mention removing this test. This removal causes a
loss of test coverage that previously verified an atomic replace successfully
disables multiple existing livepatches.

Additionally, removing this test leaves the script variables MOD_LIVEPATCH2
and MOD_LIVEPATCH3 unused in test-livepatch.sh. Could this test be adapted
to use replace_set instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529034542.68766-1-laoar.shao@gmail.com?part=4

^ permalink raw reply

* Re: [PATCH v2 3/4] livepatch: deprecate stack_order
From: sashiko-bot @ 2026-05-29  4:54 UTC (permalink / raw)
  To: song, jpoimboe, joe.lawrence, jikos, mbenes, pmladek
  Cc: live-patching, Yafang Shao
In-Reply-To: <20260529034542.68766-4-laoar.shao@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The commit entirely removes the `stack_order` sysfs attribute while only marking it as `<deprecated>` in the ABI documentation, causing immediate backwards compatibility breakage and contradicting documentation guidelines.
--

commit 041b2dbba1a7397626104dd773fc1f37228d382c
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: deprecate stack_order

This commit deprecates the stack_order sysfs attribute for atomic-replace
livepatches, since a single function can only be modified by a unique
replace_set.

> diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
> index 6d75235a6a2e4..fddad93dba797 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-livepatch
> +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
> @@ -62,6 +62,7 @@ Description:
>  		are applied to the system. If multiple live patches modify the same
>  		function, the implementation with the biggest 'stack_order' number
>  		is used, unless a transition is currently in progress.
> +		<deprecated>

[ ... ]

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 969fea2a92631..6d65f839f4420 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
[ ... ]
> @@ -457,38 +456,15 @@ static ssize_t replace_set_show(struct kobject *kobj,
>  	return sysfs_emit(buf, "%u\n", patch->replace_set);
>  }
>  
> -static ssize_t stack_order_show(struct kobject *kobj,
> -				struct kobj_attribute *attr, char *buf)
> -{
> -	struct klp_patch *patch, *this_patch;
> -	int stack_order = 0;
> -
> -	this_patch = container_of(kobj, struct klp_patch, kobj);
> -
> -	mutex_lock(&klp_mutex);
> -
> -	klp_for_each_patch(patch) {
> -		stack_order++;
> -		if (patch == this_patch)
> -			break;
> -	}
> -
> -	mutex_unlock(&klp_mutex);
> -
> -	return sysfs_emit(buf, "%d\n", stack_order);
> -}
> -
>  static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
>  static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
>  static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
>  static struct kobj_attribute replace_set_kobj_attr = __ATTR_RO(replace_set);
> -static struct kobj_attribute stack_order_kobj_attr = __ATTR_RO(stack_order);
>  static struct attribute *klp_patch_attrs[] = {
>  	&enabled_kobj_attr.attr,
>  	&transition_kobj_attr.attr,
>  	&force_kobj_attr.attr,
>  	&replace_set_kobj_attr.attr,
> -	&stack_order_kobj_attr.attr,
>  	NULL
>  };

[Severity: Critical]
The commit message and documentation state that the stack_order attribute
is being deprecated, but this code completely removes the attribute. Does
this abruptly break ABI compatibility for userspace tools that expect this
sysfs file to exist?

Normally, deprecating an interface requires keeping it around for a
transition period (perhaps returning a dummy value) rather than deleting
it immediately. Also, if it were actually removed, shouldn't the ABI
documentation be moved to the removed/ directory instead of just appending
<deprecated> to the existing entry?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529034542.68766-1-laoar.shao@gmail.com?part=3

^ permalink raw reply

* Re: [PATCH v2 2/4] livepatch: Support scoped atomic replace using replace_set
From: sashiko-bot @ 2026-05-29  4:32 UTC (permalink / raw)
  To: jikos, pmladek, song, mbenes, jpoimboe, joe.lawrence
  Cc: Yafang Shao, live-patching
In-Reply-To: <20260529034542.68766-3-laoar.shao@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A malformed livepatch module with a missing function `old_name` will cause a kernel panic during patch compatibility checking.
--

commit 48dd0b716d1f70f13c1e96b75bf54e1aba77be18
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: Support scoped atomic replace using replace_set

Convert the replace attribute from a boolean to a u32 to function as a
"replace set." A newly loaded livepatch will now atomically replace any
existing patch belonging to the same set. There can only ever be one active
livepatch for a given replace_set number.

Link: https://github.com/pmladek/linux/tree/klp-state-transfer-v1-iter12 [0]

> diff --git a/kernel/livepatch/state.c b/kernel/livepatch/state.c
> index 2565d039ade0a..a1ac466373360 100644
> --- a/kernel/livepatch/state.c
> +++ b/kernel/livepatch/state.c

[ ... ]

> @@ -85,34 +85,65 @@ EXPORT_SYMBOL_GPL(klp_get_prev_state);

[ ... ]

> +		klp_for_each_object_static(patch, obj) {
> +			klp_for_each_object(old_patch, old_obj) {
> +				if (!!obj->name != !!old_obj->name)
> +					continue;
> +				if (obj->name && strcmp(obj->name, old_obj->name))
> +					continue;
> +				klp_for_each_func_static(obj, func) {
> +					if (klp_find_func(old_obj, func))
> +						return false;
> +				}
> +			}
> +		}

[Severity: High]
Could a malformed livepatch module cause a kernel panic here during patch
compatibility checking?

Since klp_is_patch_compatible() is called in klp_enable_patch() before
klp_init_patch() validates the incoming patch structures, a malformed patch
might have func->old_name set to NULL but func->new_func populated.

In that scenario, klp_for_each_func_static() would still process the function
and pass it to klp_find_func(), where it is used as old_func:

kernel/livepatch/core.c:klp_find_func() {
    ...
    klp_for_each_func(obj, func) {
        if ((strcmp(old_func->old_name, func->old_name) == 0) &&
    ...
}

Would this result in a NULL pointer dereference when klp_find_func() attempts
to call strcmp() on the unvalidated function's old_name? While livepatch
modules are loaded by root, should they fail validation gracefully instead of
crashing the kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529034542.68766-1-laoar.shao@gmail.com?part=2

^ permalink raw reply

* Re: [PATCH v2 1/4] livepatch: Make klp_find_func() non-static
From: sashiko-bot @ 2026-05-29  4:01 UTC (permalink / raw)
  To: jikos, joe.lawrence, song, mbenes, jpoimboe, pmladek
  Cc: live-patching, Yafang Shao
In-Reply-To: <20260529034542.68766-2-laoar.shao@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] klp_find_func() is improperly exposed in the global public header, breaking encapsulation and introducing potential build/linkage errors.

Pre-existing issues:
- [High] klp_find_func() lacks input validation, exacerbating a pre-existing NULL pointer dereference vulnerability during livepatch loading.
--

commit 0b93025e6e1dd48057bb98e4d5e13741daab118e
Author: Yafang Shao <laoar.shao@gmail.com>

livepatch: Make klp_find_func() non-static
    
This commit exposes klp_find_func() by moving it out of the static scope
to make it available outside of core.c for reuse in upcoming patches.

> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index ba9e3988c07c3..70854f542c33e 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -215,6 +215,10 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
>  			     unsigned int symindex, unsigned int secindex,
>  			     const char *objname);
>  
> +struct klp_func *klp_find_func(struct klp_object *obj,
> +			       struct klp_func *old_func);
> +

[Severity: Low]
Does this declaration belong in kernel/livepatch/core.h instead? It lacks
an EXPORT_SYMBOL_GPL() and a stub for when !CONFIG_LIVEPATCH is set.

[ ... ]

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 28d15ba58a26d..e97df3e59057b 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -82,8 +82,8 @@ static bool klp_initialized(void)
>  	return !!klp_root_kobj;
>  }
>  
> -static struct klp_func *klp_find_func(struct klp_object *obj,
> -				      struct klp_func *old_func)
> +struct klp_func *klp_find_func(struct klp_object *obj,
> +			       struct klp_func *old_func)
>  {
>  	struct klp_func *func;

[Severity: High]
This is a pre-existing issue, but does this function dereference a NULL
pointer if old_name is NULL?

If a newly loaded livepatch provides a function entry with a NULL old_name,
func->old_name will be NULL when evaluated in strcmp():

klp_init_patch()
    klp_add_nops()
        klp_find_func()
            strcmp(old_func->old_name, func->old_name)

Could a check be added for NULL before calling strcmp()?

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

^ permalink raw reply

* [PATCH v2 4/4] selftests/livepatch: Update tests for replace_set
From: Yafang Shao @ 2026-05-29  3:45 UTC (permalink / raw)
  To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, song
  Cc: live-patching, Yafang Shao
In-Reply-To: <20260529034542.68766-1-laoar.shao@gmail.com>

The changes are as follows:
- Replace "replace" with "replace_set"
- For atomic-replace test cases, we configure two different livepatches
  that share the same replace_set.
- For non-atomic-replace test cases, we configure multiple livepatches,
  each with a different replace_set.

The result are as follows,

$ ./test-callbacks.sh
TEST: target module before livepatch ... ok
TEST: module_coming notifier ... ok
TEST: module_going notifier ... ok
TEST: module_coming and module_going notifiers ... ok
TEST: target module not present ... ok
TEST: pre-patch callback -ENODEV ... ok
TEST: module_coming + pre-patch callback -ENODEV ... ok
TEST: multiple target modules ... ok
TEST: busy target module ... ok
TEST: multiple livepatches ... ok
TEST: atomic replace ... ok

$ ./test-ftrace.sh
TEST: livepatch interaction with ftrace_enabled sysctl ... ok
TEST: trace livepatched function and check that the live patch remains in effect ... ok
TEST: livepatch a traced function and check that the live patch remains in effect ... ok

$ ./test-kprobe.sh
TEST: livepatch interaction with kprobed function with post_handler ... ok
TEST: livepatch interaction with kprobed function without post_handler ... ok

$ ./test-livepatch.sh
TEST: basic function patching ... ok
TEST: multiple livepatches ... ok
TEST: module function patching ... ok
TEST: module function patching (livepatch first) ... ok

$ ./test-shadow-vars.sh
TEST: basic shadow variable API ... ok

$ ./test-state.sh
TEST: system state modification ... ok
TEST: taking over system state modification ... ok
TEST: compatible cumulative livepatches ... ok
TEST: incompatible cumulative livepatches ... ok

$ ./test-syscall.sh
TEST: patch getpid syscall while being heavily hammered ... ok

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 .../selftests/livepatch/test-callbacks.sh     | 33 +++----
 .../selftests/livepatch/test-livepatch.sh     | 98 +------------------
 .../testing/selftests/livepatch/test-sysfs.sh | 22 ++---
 .../test_modules/test_klp_atomic_replace.c    | 10 +-
 .../test_modules/test_klp_callbacks_demo.c    |  6 ++
 .../test_modules/test_klp_callbacks_demo2.c   | 10 +-
 .../test_modules/test_klp_livepatch.c         |  6 ++
 .../livepatch/test_modules/test_klp_state.c   |  2 +-
 .../livepatch/test_modules/test_klp_state2.c  |  2 +-
 9 files changed, 55 insertions(+), 134 deletions(-)

diff --git a/tools/testing/selftests/livepatch/test-callbacks.sh b/tools/testing/selftests/livepatch/test-callbacks.sh
index 2a03deb26a12..692da8ea4c25 100755
--- a/tools/testing/selftests/livepatch/test-callbacks.sh
+++ b/tools/testing/selftests/livepatch/test-callbacks.sh
@@ -451,8 +451,8 @@ $MOD_TARGET_BUSY: busymod_work_func exit
 $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
 
 
-# Test loading multiple livepatches.  This test-case is mainly for comparing
-# with the next test-case.
+# Test loading multiple livepatches sharing different replace_set.
+# This test-case is mainly for comparing with the next test-case.
 #
 # - Load and unload two livepatches, pre and post (un)patch callbacks
 #   execute as each patch progresses through its (un)patching
@@ -460,14 +460,14 @@ $MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
 
 start_test "multiple livepatches"
 
-load_lp $MOD_LIVEPATCH
-load_lp $MOD_LIVEPATCH2
+load_lp $MOD_LIVEPATCH replace_set=0
+load_lp $MOD_LIVEPATCH2 replace_set=1
 disable_lp $MOD_LIVEPATCH2
 disable_lp $MOD_LIVEPATCH
 unload_lp $MOD_LIVEPATCH2
 unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace_set=0
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
 $MOD_LIVEPATCH: pre_patch_callback: vmlinux
@@ -475,7 +475,7 @@ livepatch: '$MOD_LIVEPATCH': starting patching transition
 livepatch: '$MOD_LIVEPATCH': completing patching transition
 $MOD_LIVEPATCH: post_patch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH': patching complete
-% insmod test_modules/$MOD_LIVEPATCH2.ko
+% insmod test_modules/$MOD_LIVEPATCH2.ko replace_set=1
 livepatch: enabling patch '$MOD_LIVEPATCH2'
 livepatch: '$MOD_LIVEPATCH2': initializing patching transition
 $MOD_LIVEPATCH2: pre_patch_callback: vmlinux
@@ -501,14 +501,13 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
 % rmmod $MOD_LIVEPATCH"
 
 
-# Load multiple livepatches, but the second as an 'atomic-replace'
-# patch.  When the latter loads, the original livepatch should be
-# disabled and *none* of its pre/post-unpatch callbacks executed.  On
-# the other hand, when the atomic-replace livepatch is disabled, its
-# pre/post-unpatch callbacks *should* be executed.
+# Load multiple livepatches sharing the same replace_set.
+# When the latter loads, the original livepatch should be disabled and
+# *none* of its pre/post-unpatch callbacks executed.  On the other hand,
+# when the atomic-replace livepatch is disabled, its pre/post-unpatch
+# callbacks *should* be executed.
 #
-# - Load and unload two livepatches, the second of which has its
-#   .replace flag set true.
+# - Load and unload two livepatches sharing the same replace_set
 #
 # - Pre and post patch callbacks are executed for both livepatches.
 #
@@ -517,13 +516,13 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
 
 start_test "atomic replace"
 
-load_lp $MOD_LIVEPATCH
-load_lp $MOD_LIVEPATCH2 replace=1
+load_lp $MOD_LIVEPATCH replace_set=0
+load_lp $MOD_LIVEPATCH2 replace_set=0
 disable_lp $MOD_LIVEPATCH2
 unload_lp $MOD_LIVEPATCH2
 unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace_set=0
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
 $MOD_LIVEPATCH: pre_patch_callback: vmlinux
@@ -531,7 +530,7 @@ livepatch: '$MOD_LIVEPATCH': starting patching transition
 livepatch: '$MOD_LIVEPATCH': completing patching transition
 $MOD_LIVEPATCH: post_patch_callback: vmlinux
 livepatch: '$MOD_LIVEPATCH': patching complete
-% insmod test_modules/$MOD_LIVEPATCH2.ko replace=1
+% insmod test_modules/$MOD_LIVEPATCH2.ko replace_set=0
 livepatch: enabling patch '$MOD_LIVEPATCH2'
 livepatch: '$MOD_LIVEPATCH2': initializing patching transition
 $MOD_LIVEPATCH2: pre_patch_callback: vmlinux
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index c44c5341a2f1..042999856267 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -57,12 +57,12 @@ livepatch: '$MOD_LIVEPATCH1': unpatching complete
 
 start_test "multiple livepatches"
 
-load_lp $MOD_LIVEPATCH1
+load_lp $MOD_LIVEPATCH1 replace_set=0
 
 grep 'live patched' /proc/cmdline > /dev/kmsg
 grep 'live patched' /proc/meminfo > /dev/kmsg
 
-load_lp $MOD_REPLACE replace=0
+load_lp $MOD_REPLACE replace_set=1
 
 grep 'live patched' /proc/cmdline > /dev/kmsg
 grep 'live patched' /proc/meminfo > /dev/kmsg
@@ -79,14 +79,14 @@ unload_lp $MOD_LIVEPATCH1
 grep 'live patched' /proc/cmdline > /dev/kmsg
 grep 'live patched' /proc/meminfo > /dev/kmsg
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH1.ko
+check_result "% insmod test_modules/$MOD_LIVEPATCH1.ko replace_set=0
 livepatch: enabling patch '$MOD_LIVEPATCH1'
 livepatch: '$MOD_LIVEPATCH1': initializing patching transition
 livepatch: '$MOD_LIVEPATCH1': starting patching transition
 livepatch: '$MOD_LIVEPATCH1': completing patching transition
 livepatch: '$MOD_LIVEPATCH1': patching complete
 $MOD_LIVEPATCH1: this has been live patched
-% insmod test_modules/$MOD_REPLACE.ko replace=0
+% insmod test_modules/$MOD_REPLACE.ko replace_set=1
 livepatch: enabling patch '$MOD_REPLACE'
 livepatch: '$MOD_REPLACE': initializing patching transition
 livepatch: '$MOD_REPLACE': starting patching transition
@@ -108,96 +108,6 @@ livepatch: '$MOD_LIVEPATCH1': completing unpatching transition
 livepatch: '$MOD_LIVEPATCH1': unpatching complete
 % rmmod $MOD_LIVEPATCH1"
 
-
-# - load a livepatch that modifies the output from /proc/cmdline and
-#   verify correct behavior
-# - load two additional livepatches and check the number of livepatch modules
-#   applied
-# - load an atomic replace livepatch and check that the other three modules were
-#   disabled
-# - remove all livepatches besides the atomic replace one and verify that the
-#   atomic replace livepatch is still active
-# - remove the atomic replace livepatch and verify that none are active
-
-start_test "atomic replace livepatch"
-
-load_lp $MOD_LIVEPATCH1
-
-grep 'live patched' /proc/cmdline > /dev/kmsg
-grep 'live patched' /proc/meminfo > /dev/kmsg
-
-for mod in $MOD_LIVEPATCH2 $MOD_LIVEPATCH3; do
-	load_lp "$mod"
-done
-
-mods=($SYSFS_KLP_DIR/*)
-nmods=${#mods[@]}
-if [ "$nmods" -ne 3 ]; then
-	die "Expecting three modules listed, found $nmods"
-fi
-
-load_lp $MOD_REPLACE replace=1
-
-grep 'live patched' /proc/cmdline > /dev/kmsg
-grep 'live patched' /proc/meminfo > /dev/kmsg
-
-loop_until 'mods=($SYSFS_KLP_DIR/*); nmods=${#mods[@]}; [[ "$nmods" -eq 1 ]]' ||
-        die "Expecting only one moduled listed, found $nmods"
-
-# These modules were disabled by the atomic replace
-for mod in $MOD_LIVEPATCH3 $MOD_LIVEPATCH2 $MOD_LIVEPATCH1; do
-	unload_lp "$mod"
-done
-
-grep 'live patched' /proc/cmdline > /dev/kmsg
-grep 'live patched' /proc/meminfo > /dev/kmsg
-
-disable_lp $MOD_REPLACE
-unload_lp $MOD_REPLACE
-
-grep 'live patched' /proc/cmdline > /dev/kmsg
-grep 'live patched' /proc/meminfo > /dev/kmsg
-
-check_result "% insmod test_modules/$MOD_LIVEPATCH1.ko
-livepatch: enabling patch '$MOD_LIVEPATCH1'
-livepatch: '$MOD_LIVEPATCH1': initializing patching transition
-livepatch: '$MOD_LIVEPATCH1': starting patching transition
-livepatch: '$MOD_LIVEPATCH1': completing patching transition
-livepatch: '$MOD_LIVEPATCH1': patching complete
-$MOD_LIVEPATCH1: this has been live patched
-% insmod test_modules/$MOD_LIVEPATCH2.ko
-livepatch: enabling patch '$MOD_LIVEPATCH2'
-livepatch: '$MOD_LIVEPATCH2': initializing patching transition
-livepatch: '$MOD_LIVEPATCH2': starting patching transition
-livepatch: '$MOD_LIVEPATCH2': completing patching transition
-livepatch: '$MOD_LIVEPATCH2': patching complete
-% insmod test_modules/$MOD_LIVEPATCH3.ko
-livepatch: enabling patch '$MOD_LIVEPATCH3'
-livepatch: '$MOD_LIVEPATCH3': initializing patching transition
-$MOD_LIVEPATCH3: pre_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH3': starting patching transition
-livepatch: '$MOD_LIVEPATCH3': completing patching transition
-$MOD_LIVEPATCH3: post_patch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH3': patching complete
-% insmod test_modules/$MOD_REPLACE.ko replace=1
-livepatch: enabling patch '$MOD_REPLACE'
-livepatch: '$MOD_REPLACE': initializing patching transition
-livepatch: '$MOD_REPLACE': starting patching transition
-livepatch: '$MOD_REPLACE': completing patching transition
-livepatch: '$MOD_REPLACE': patching complete
-$MOD_REPLACE: this has been live patched
-% rmmod $MOD_LIVEPATCH3
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH1
-$MOD_REPLACE: this has been live patched
-% echo 0 > $SYSFS_KLP_DIR/$MOD_REPLACE/enabled
-livepatch: '$MOD_REPLACE': initializing unpatching transition
-livepatch: '$MOD_REPLACE': starting unpatching transition
-livepatch: '$MOD_REPLACE': completing unpatching transition
-livepatch: '$MOD_REPLACE': unpatching complete
-% rmmod $MOD_REPLACE"
-
-
 # - load a target module that provides /proc/test_klp_mod_target with
 #   original output
 # - load a livepatch that patches the target module's show function
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
index 0c31759f34f6..37425ad89f58 100755
--- a/tools/testing/selftests/livepatch/test-sysfs.sh
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -20,7 +20,7 @@ check_sysfs_rights "$MOD_LIVEPATCH" "" "drwxr-xr-x"
 check_sysfs_rights "$MOD_LIVEPATCH" "enabled" "-rw-r--r--"
 check_sysfs_value  "$MOD_LIVEPATCH" "enabled" "1"
 check_sysfs_rights "$MOD_LIVEPATCH" "force" "--w-------"
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
+check_sysfs_rights "$MOD_LIVEPATCH" "replace_set" "-r--r--r--"
 check_sysfs_rights "$MOD_LIVEPATCH" "transition" "-r--r--r--"
 check_sysfs_value  "$MOD_LIVEPATCH" "transition" "0"
 check_sysfs_rights "$MOD_LIVEPATCH" "vmlinux/patched" "-r--r--r--"
@@ -86,18 +86,18 @@ test_klp_callbacks_demo: post_unpatch_callback: vmlinux
 livepatch: 'test_klp_callbacks_demo': unpatching complete
 % rmmod test_klp_callbacks_demo"
 
-start_test "sysfs test replace enabled"
+start_test "sysfs test replace_set 0"
 
 MOD_LIVEPATCH=test_klp_atomic_replace
-load_lp $MOD_LIVEPATCH replace=1
+load_lp $MOD_LIVEPATCH replace_set=0
 
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
-check_sysfs_value  "$MOD_LIVEPATCH" "replace" "1"
+check_sysfs_rights "$MOD_LIVEPATCH" "replace_set" "-r--r--r--"
+check_sysfs_value  "$MOD_LIVEPATCH" "replace_set" "0"
 
 disable_lp $MOD_LIVEPATCH
 unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=1
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace_set=0
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
 livepatch: '$MOD_LIVEPATCH': starting patching transition
@@ -110,17 +110,17 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
 livepatch: '$MOD_LIVEPATCH': unpatching complete
 % rmmod $MOD_LIVEPATCH"
 
-start_test "sysfs test replace disabled"
+start_test "sysfs test replace_set 1234"
 
-load_lp $MOD_LIVEPATCH replace=0
+load_lp $MOD_LIVEPATCH replace_set=1234
 
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
-check_sysfs_value  "$MOD_LIVEPATCH" "replace" "0"
+check_sysfs_rights "$MOD_LIVEPATCH" "replace_set" "-r--r--r--"
+check_sysfs_value  "$MOD_LIVEPATCH" "replace_set" "1234"
 
 disable_lp $MOD_LIVEPATCH
 unload_lp $MOD_LIVEPATCH
 
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=0
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace_set=1234
 livepatch: enabling patch '$MOD_LIVEPATCH'
 livepatch: '$MOD_LIVEPATCH': initializing patching transition
 livepatch: '$MOD_LIVEPATCH': starting patching transition
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c b/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c
index 5af7093ca00c..5333503f193a 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_atomic_replace.c
@@ -7,9 +7,9 @@
 #include <linux/kernel.h>
 #include <linux/livepatch.h>
 
-static int replace;
-module_param(replace, int, 0644);
-MODULE_PARM_DESC(replace, "replace (default=0)");
+static int replace_set;
+module_param(replace_set, int, 0644);
+MODULE_PARM_DESC(replace_set, "replace_set (default=0)");
 
 #include <linux/seq_file.h>
 static int livepatch_meminfo_proc_show(struct seq_file *m, void *v)
@@ -36,12 +36,12 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
-	/* set .replace in the init function below for demo purposes */
+	/* set .replace_set in the init function below for demo purposes */
 };
 
 static int test_klp_atomic_replace_init(void)
 {
-	patch.replace = replace;
+	patch.replace_set = replace_set;
 	return klp_enable_patch(&patch);
 }
 
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c b/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c
index 3fd8fe1cd1cc..5c3324aa4d75 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo.c
@@ -7,6 +7,10 @@
 #include <linux/kernel.h>
 #include <linux/livepatch.h>
 
+static int replace_set;
+module_param(replace_set, int, 0644);
+MODULE_PARM_DESC(replace_set, "replace_set (default=0)");
+
 static int pre_patch_ret;
 module_param(pre_patch_ret, int, 0644);
 MODULE_PARM_DESC(pre_patch_ret, "pre_patch_ret (default=0)");
@@ -102,10 +106,12 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
+	/* set .replace_set in the init function below for demo purposes */
 };
 
 static int test_klp_callbacks_demo_init(void)
 {
+	patch.replace_set = replace_set;
 	return klp_enable_patch(&patch);
 }
 
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c b/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c
index 5417573e80af..31347e2131a7 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_callbacks_demo2.c
@@ -7,9 +7,9 @@
 #include <linux/kernel.h>
 #include <linux/livepatch.h>
 
-static int replace;
-module_param(replace, int, 0644);
-MODULE_PARM_DESC(replace, "replace (default=0)");
+static int replace_set;
+module_param(replace_set, int, 0644);
+MODULE_PARM_DESC(replace_set, "replace_set (default=0)");
 
 static const char *const module_state[] = {
 	[MODULE_STATE_LIVE]	= "[MODULE_STATE_LIVE] Normal state",
@@ -72,12 +72,12 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
-	/* set .replace in the init function below for demo purposes */
+	/* set .replace_set in the init function below for demo purposes */
 };
 
 static int test_klp_callbacks_demo2_init(void)
 {
-	patch.replace = replace;
+	patch.replace_set = replace_set;
 	return klp_enable_patch(&patch);
 }
 
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c b/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c
index aff08199de71..fedd2494d187 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_livepatch.c
@@ -15,6 +15,10 @@ static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int replace_set;
+module_param(replace_set, int, 0644);
+MODULE_PARM_DESC(replace_set, "replace_set (default=0)");
+
 static struct klp_func funcs[] = {
 	{
 		.old_name = "cmdline_proc_show",
@@ -32,10 +36,12 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
+	/* set .replace_set in the init function below for demo purposes */
 };
 
 static int test_klp_livepatch_init(void)
 {
+	patch.replace_set = replace_set;
 	return klp_enable_patch(&patch);
 }
 
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
index 57a4253acb01..8c8829c3ec43 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
@@ -142,7 +142,7 @@ static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
 	.states = states,
-	.replace = true,
+	.replace_set = 0,
 };
 
 static int test_klp_callbacks_demo_init(void)
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c b/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
index c978ea4d5e67..8a79d7dcce33 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state2.c
@@ -171,7 +171,7 @@ static struct klp_patch patch = {
 	.mod = THIS_MODULE,
 	.objs = objs,
 	.states = states,
-	.replace = true,
+	.replace_set = 0,
 };
 
 static int test_klp_callbacks_demo_init(void)
-- 
2.47.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox