* [PATCH v2 11/12] objtool: Introduce objtool for arm64
From: Josh Poimboeuf @ 2026-03-17 22:51 UTC (permalink / raw)
To: x86
Cc: linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Nicolas Schier, Herbert Xu
In-Reply-To: <cover.1773787568.git.jpoimboe@kernel.org>
Add basic support for arm64 on objtool. Only --checksum is currently
supported.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
arch/arm64/Kconfig | 2 +
tools/objtool/Makefile | 4 +
tools/objtool/arch/arm64/Build | 2 +
tools/objtool/arch/arm64/decode.c | 116 ++++++++++++++++++
.../arch/arm64/include/arch/cfi_regs.h | 11 ++
tools/objtool/arch/arm64/include/arch/elf.h | 13 ++
.../objtool/arch/arm64/include/arch/special.h | 21 ++++
tools/objtool/arch/arm64/special.c | 21 ++++
8 files changed, 190 insertions(+)
create mode 100644 tools/objtool/arch/arm64/Build
create mode 100644 tools/objtool/arch/arm64/decode.c
create mode 100644 tools/objtool/arch/arm64/include/arch/cfi_regs.h
create mode 100644 tools/objtool/arch/arm64/include/arch/elf.h
create mode 100644 tools/objtool/arch/arm64/include/arch/special.h
create mode 100644 tools/objtool/arch/arm64/special.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 38dba5f7e4d2..354aa80c5b4b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -236,9 +236,11 @@ config ARM64
select HAVE_HW_BREAKPOINT if PERF_EVENTS
select HAVE_IOREMAP_PROT
select HAVE_IRQ_TIME_ACCOUNTING
+ select HAVE_KLP_BUILD
select HAVE_LIVEPATCH
select HAVE_MOD_ARCH_SPECIFIC
select HAVE_NMI
+ select HAVE_OBJTOOL
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
select HAVE_PERF_REGS
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index b71d1886022e..66b04172b79a 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -11,6 +11,10 @@ ifeq ($(SRCARCH),loongarch)
BUILD_ORC := y
endif
+ifeq ($(SRCARCH),arm64)
+ ARCH_HAS_KLP := y
+endif
+
ifeq ($(ARCH_HAS_KLP),y)
HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
$(HOSTCC) $(HOSTCFLAGS) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
new file mode 100644
index 000000000000..d24d5636a5b8
--- /dev/null
+++ b/tools/objtool/arch/arm64/Build
@@ -0,0 +1,2 @@
+objtool-y += decode.o
+objtool-y += special.o
diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c
new file mode 100644
index 000000000000..ee93c096243f
--- /dev/null
+++ b/tools/objtool/arch/arm64/decode.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <objtool/check.h>
+#include <objtool/disas.h>
+#include <objtool/elf.h>
+#include <objtool/arch.h>
+#include <objtool/warn.h>
+#include <objtool/builtin.h>
+
+const char *arch_reg_name[CFI_NUM_REGS] = {};
+
+int arch_ftrace_match(const char *name)
+{
+ return 0;
+}
+
+s64 arch_insn_adjusted_addend(struct instruction *insn, struct reloc *reloc)
+{
+ return reloc_addend(reloc);
+}
+
+bool arch_callee_saved_reg(unsigned char reg)
+{
+ return false;
+}
+
+int arch_decode_hint_reg(u8 sp_reg, int *base)
+{
+ exit(-1);
+}
+
+const char *arch_nop_insn(int len)
+{
+ exit(-1);
+}
+
+const char *arch_ret_insn(int len)
+{
+ exit(-1);
+}
+
+int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
+ unsigned long offset, unsigned int maxlen,
+ struct instruction *insn)
+{
+ u32 i = *((u32 *)(sec->data->d_buf + offset));
+
+ insn->len = 4;
+
+ /*
+ * These are the bare minimum needed for static branch detection and
+ * checksum calculations.
+ */
+ if (i == 0xd503201f || i == 0x2a1f03f7) {
+ /* For static branches */
+ insn->type = INSN_NOP;
+ } else if ((i & 0xfc000000) == 0x14000000) {
+ /* For static branches and intra-TU sibling calls */
+ insn->type = INSN_JUMP_UNCONDITIONAL;
+ insn->immediate = sign_extend64(i & 0x03ffffff, 25);
+ } else if ((i & 0xfc000000) == 0x94000000) {
+ /* For intra-TU calls */
+ insn->type = INSN_CALL;
+ insn->immediate = sign_extend64(i & 0x03ffffff, 25);
+ } else if ((i & 0xff000000) == 0x54000000) {
+ /* For intra-TU conditional sibling calls */
+ insn->type = INSN_JUMP_CONDITIONAL;
+ insn->immediate = sign_extend64((i & 0xffffe0) >> 5, 18);
+ } else {
+ insn->type = INSN_OTHER;
+ }
+
+ return 0;
+}
+
+u64 arch_adjusted_addend(struct reloc *reloc)
+{
+ return reloc_addend(reloc);
+}
+
+unsigned long arch_jump_destination(struct instruction *insn)
+{
+ return insn->offset + (insn->immediate << 2);
+}
+
+bool arch_pc_relative_reloc(struct reloc *reloc)
+{
+ return false;
+}
+
+void arch_initial_func_cfi_state(struct cfi_init_state *state)
+{
+ state->cfa.base = CFI_UNDEFINED;
+}
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+ switch (reloc_type(reloc)) {
+ case R_AARCH64_ABS64:
+ case R_AARCH64_PREL64:
+ return 8;
+ default:
+ return 4;
+ }
+}
+
+#ifdef DISAS
+int arch_disas_info_init(struct disassemble_info *dinfo)
+{
+ return disas_info_init(dinfo, bfd_arch_aarch64,
+ bfd_mach_arm_unknown, bfd_mach_aarch64,
+ NULL);
+}
+#endif /* DISAS */
diff --git a/tools/objtool/arch/arm64/include/arch/cfi_regs.h b/tools/objtool/arch/arm64/include/arch/cfi_regs.h
new file mode 100644
index 000000000000..49c81cbb6646
--- /dev/null
+++ b/tools/objtool/arch/arm64/include/arch/cfi_regs.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_CFI_REGS_H
+#define _OBJTOOL_ARCH_CFI_REGS_H
+
+/* These aren't actually used for arm64 */
+#define CFI_BP 0
+#define CFI_SP 0
+#define CFI_RA 0
+#define CFI_NUM_REGS 2
+
+#endif /* _OBJTOOL_ARCH_CFI_REGS_H */
diff --git a/tools/objtool/arch/arm64/include/arch/elf.h b/tools/objtool/arch/arm64/include/arch/elf.h
new file mode 100644
index 000000000000..80a1bc479930
--- /dev/null
+++ b/tools/objtool/arch/arm64/include/arch/elf.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_ELF_H
+#define _OBJTOOL_ARCH_ELF_H
+
+#define R_NONE R_AARCH64_NONE
+#define R_ABS64 R_AARCH64_ABS64
+#define R_ABS32 R_AARCH64_ABS32
+#define R_DATA32 R_AARCH64_PREL32
+#define R_DATA64 R_AARCH64_PREL64
+#define R_TEXT32 R_AARCH64_PREL32
+#define R_TEXT64 R_AARCH64_PREL64
+
+#endif /* _OBJTOOL_ARCH_ELF_H */
diff --git a/tools/objtool/arch/arm64/include/arch/special.h b/tools/objtool/arch/arm64/include/arch/special.h
new file mode 100644
index 000000000000..8ae804a83ea4
--- /dev/null
+++ b/tools/objtool/arch/arm64/include/arch/special.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_SPECIAL_H
+#define _OBJTOOL_ARCH_SPECIAL_H
+
+#define EX_ENTRY_SIZE 12
+#define EX_ORIG_OFFSET 0
+#define EX_NEW_OFFSET 4
+
+#define JUMP_ENTRY_SIZE 16
+#define JUMP_ORIG_OFFSET 0
+#define JUMP_NEW_OFFSET 4
+#define JUMP_KEY_OFFSET 8
+
+#define ALT_ENTRY_SIZE 12
+#define ALT_ORIG_OFFSET 0
+#define ALT_NEW_OFFSET 4
+#define ALT_FEATURE_OFFSET 8
+#define ALT_ORIG_LEN_OFFSET 10
+#define ALT_NEW_LEN_OFFSET 11
+
+#endif /* _OBJTOOL_ARCH_SPECIAL_H */
diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c
new file mode 100644
index 000000000000..3c2b83d94939
--- /dev/null
+++ b/tools/objtool/arch/arm64/special.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <objtool/special.h>
+
+bool arch_support_alt_relocation(struct special_alt *special_alt,
+ struct instruction *insn,
+ struct reloc *reloc)
+{
+ return false;
+}
+
+struct reloc *arch_find_switch_table(struct objtool_file *file,
+ struct instruction *insn,
+ unsigned long *table_size)
+{
+ return NULL;
+}
+
+const char *arch_cpu_feature_name(int feature_number)
+{
+ return NULL;
+}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 12/12] klp-build: Support cross-compilation
From: Josh Poimboeuf @ 2026-03-17 22:51 UTC (permalink / raw)
To: x86
Cc: linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Nicolas Schier, Herbert Xu
In-Reply-To: <cover.1773787568.git.jpoimboe@kernel.org>
Add support for cross-compilation. The user must export ARCH, and
either CROSS_COMPILE or LLVM as needed.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
scripts/livepatch/klp-build | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 809e198a561d..b6c057e2120f 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -404,6 +404,14 @@ validate_patches() {
revert_patches
}
+cross_compile_init() {
+ if [[ -v LLVM ]]; then
+ OBJCOPY=llvm-objcopy
+ else
+ OBJCOPY="${CROSS_COMPILE:-}objcopy"
+ fi
+}
+
do_init() {
# We're not yet smart enough to handle anything other than in-tree
# builds in pwd.
@@ -420,6 +428,7 @@ do_init() {
validate_config
set_module_name
set_kernelversion
+ cross_compile_init
}
# Refresh the patch hunk headers, specifically the line numbers and counts.
@@ -783,7 +792,7 @@ build_patch_module() {
cp -f "$kmod_file" "$kmod_file.orig"
# Work around issue where slight .config change makes corrupt BTF
- objcopy --remove-section=.BTF "$kmod_file"
+ "$OBJCOPY" --remove-section=.BTF "$kmod_file"
# Fix (and work around) linker wreckage for klp syms / relocs
"$SRC/tools/objtool/objtool" klp post-link "$kmod_file" || die "objtool klp post-link failed"
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command
From: Nicolas Schier @ 2026-03-18 19:54 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Herbert Xu
In-Reply-To: <42418c5fa73a8876e91b3dfb38fa3f263e39f1c1.1773787568.git.jpoimboe@kernel.org>
On Tue, Mar 17, 2026 at 03:51:07PM -0700, Josh Poimboeuf wrote:
> Split the objtool args into commands and options, such that if no
> commands have been enabled, objtool doesn't run.
>
> This is in preparation in enabling objtool and klp-build for arm64.
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> arch/x86/boot/startup/Makefile | 2 +-
> scripts/Makefile.build | 4 +--
> scripts/Makefile.lib | 46 ++++++++++++++++++----------------
> scripts/Makefile.vmlinux_o | 15 ++++-------
> 4 files changed, 33 insertions(+), 34 deletions(-)
>
[...]
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3652b85be545..8a1bdfdb2fdb 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
>
> ifdef CONFIG_OBJTOOL
> -$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
> +$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))
Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if
$(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be
left-out:
$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))
> endif
>
> ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
> @@ -501,7 +501,7 @@ define rule_ld_multi_m
> $(call cmd,gen_objtooldep)
> endef
>
> -$(multi-obj-m): private objtool-enabled := $(delay-objtool)
> +$(multi-obj-m): private objtool-enabled := $(if $(objtool-cmds-y),$(delay-objtool))
Could be changed to $(and), too; but personally I think the $(if) is
easier to parse at once.
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command
From: Josh Poimboeuf @ 2026-03-19 0:49 UTC (permalink / raw)
To: Nicolas Schier
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Herbert Xu
In-Reply-To: <absC93h6fNgyniD4@derry.ads.avm.de>
On Wed, Mar 18, 2026 at 08:54:31PM +0100, Nicolas Schier wrote:
> On Tue, Mar 17, 2026 at 03:51:07PM -0700, Josh Poimboeuf wrote:
> > Split the objtool args into commands and options, such that if no
> > commands have been enabled, objtool doesn't run.
> >
> > This is in preparation in enabling objtool and klp-build for arm64.
> >
> > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > ---
> > arch/x86/boot/startup/Makefile | 2 +-
> > scripts/Makefile.build | 4 +--
> > scripts/Makefile.lib | 46 ++++++++++++++++++----------------
> > scripts/Makefile.vmlinux_o | 15 ++++-------
> > 4 files changed, 33 insertions(+), 34 deletions(-)
> >
> [...]
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 3652b85be545..8a1bdfdb2fdb 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> > is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
> >
> > ifdef CONFIG_OBJTOOL
> > -$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
> > +$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))
>
> Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if
> $(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be
> left-out:
>
> $(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))
I believe that would break the !delay-objtool case. The logic needs to
be something like:
if (is-standard-object && objtool-cmds-y) {
if (delay-objtool) {
// for delay-objtool, only enable objtool for single-object modules
$(is-single-obj-m)
} else {
// for !delay-objtool, always enable objtool
y
}
}
so maybe something like this?
$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y))
--
Josh
^ permalink raw reply
* Re: [PATCH v2 00/12] objtool/arm64: Port klp-build to arm64
From: Josh Poimboeuf @ 2026-03-19 4:13 UTC (permalink / raw)
To: x86
Cc: linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Nicolas Schier, Herbert Xu
In-Reply-To: <cover.1773787568.git.jpoimboe@kernel.org>
On Tue, Mar 17, 2026 at 03:51:00PM -0700, Josh Poimboeuf wrote:
> v2:
> - patches 1-2 were merged, rebase on tip/master
> - improve commit message for "objtool: Extricate checksum calculation from validate_branch()"
> - add review tags
>
> v1: https://lore.kernel.org/cover.1772681234.git.jpoimboe@kernel.org
>
> Port objtool and the klp-build tooling (for building livepatch modules)
> to arm64.
>
> Note this doesn't bring all the objtool bells and whistles to arm64, nor
> any of the CFG reverse engineering. This only adds the bare minimum
> needed for 'objtool --checksum'.
>
> And note that objtool still doesn't get enabled at all for normal arm64
> kernel builds, so this doesn't affect any users other than those running
> klp-build directly.
The Sashiko AI thing found some bugs, so it might be wise to hold off on
any review or testing of this until I get v3 out.
--
Josh
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Miroslav Benes @ 2026-03-19 12:54 UTC (permalink / raw)
To: Joe Lawrence
Cc: Marcos Paulo de Souza, Josh Poimboeuf, Jiri Kosina, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <abhjYtyveer4niGM@redhat.com>
On Mon, 16 Mar 2026, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza wrote:
> > Instead of checking if the architecture running the test was powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
There is a typo...
s/CONF_ARCH_HAS_SYSCALL_WRAPPER/CONFIG_ARCH_HAS_SYSCALL_WRAPPER/
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > 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 dd802783ea849..c01a586866304 100644
> > --- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE && !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
I would take a bit further. We would rely on
CONFIG_ARCH_HAS_SYSCALL_WRAPPER being set/unset per listed architectures
"correctly" for us. If it changes somehow (though I cannot imagine reasons
for that but let's say we add new architecture. LoongArch also supports
live patching.), the above might evaluate to something broken.
So I would perhaps prefer to stay with the logic that defines FN_PREFIX
per architecture and has also #else branch for the rest. And more comments
never hurt.
Btw, see also
https://sashiko.dev/#/patchset/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253%40suse.com
for the Sashiko AI review. It also commented on this patch. Marcos, I
guess that you will look there and I will just omit what Sashiko found in
my review if I spot the same thing.
Miroslav
^ permalink raw reply
* Re: [PATCH 2/8] selftests: livepatch: test-kprobe: Replace true/false mod param by 1/0
From: Miroslav Benes @ 2026-03-19 13:03 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-2-71ac6dfb3253@suse.com>
A nit but I think that "test-kprobe: " is unnecessary noise in the subject
and can be dropped. It applies to all patches in the series.
On Fri, 13 Mar 2026, Marcos Paulo de Souza wrote:
> Older kernels don't support true/false for boolean module parameters
> because they lack commit 0d6ea3ac94ca
> ("lib/kstrtox.c: add "false"/"true" support to kstrtobool()"). Replace
> true/false by 1/0 so the test module can be loaded on older kernels.
>
> No functional changes.
We also define a bool module parameter in
test_modules/test_klp_callbacks_busy.c. Does it have a similar problem?
Miroslav
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Marcos Paulo de Souza @ 2026-03-19 14:11 UTC (permalink / raw)
To: Miroslav Benes, Joe Lawrence
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Shuah Khan,
live-patching, linux-kselftest, linux-kernel
In-Reply-To: <alpine.LSU.2.21.2603191349440.22987@pobox.suse.cz>
On Thu, 2026-03-19 at 13:54 +0100, Miroslav Benes wrote:
> On Mon, 16 Mar 2026, Joe Lawrence wrote:
>
> > On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> > wrote:
> > > Instead of checking if the architecture running the test was
> > > powerpc,
> > > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
>
> There is a typo...
> s/CONF_ARCH_HAS_SYSCALL_WRAPPER/CONFIG_ARCH_HAS_SYSCALL_WRAPPER/
Thanks, I'll fix it in my next version.
>
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > > ---
> > > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.
> > > c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > 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 dd802783ea849..c01a586866304 100644
> > > ---
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > +++
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall
> > > .c
> > > @@ -12,15 +12,14 @@
> > > #include <linux/slab.h>
> > > #include <linux/livepatch.h>
> > >
> > > -#if defined(__x86_64__)
> > > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > > +#define FN_PREFIX
> > > +#elif defined(__x86_64__)
> > > #define FN_PREFIX __x64_
> > > #elif defined(__s390x__)
> > > #define FN_PREFIX __s390x_
> > > #elif defined(__aarch64__)
> > > #define FN_PREFIX __arm64_
> > > -#else
> > > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > > -#define FN_PREFIX
> >
> > The patch does maintain the previous behavior, but I'm wondering if
> > the
> > original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> > correct:
> >
> > $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> > select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> > !COMPAT
> > depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> >
> > Perhaps I just forgot what that additional piece of information
> > that
> > explains the comment (highly probable these days), and if so, might
> > be
> > nice to add to this commit since I don't see it in 6a71770442b5
> > ("selftests: livepatch: Test livepatching a heavily called
> > syscall").
>
> I would take a bit further. We would rely on
> CONFIG_ARCH_HAS_SYSCALL_WRAPPER being set/unset per listed
> architectures
> "correctly" for us. If it changes somehow (though I cannot imagine
> reasons
> for that but let's say we add new architecture. LoongArch also
> supports
> live patching.), the above might evaluate to something broken.
>
I agree. Given that nobody even complained about it, I would say that
people testing on ppc64le has this defined correctly. Whenever new
archs start supporting livepatching, we can always revisit.
> So I would perhaps prefer to stay with the logic that defines
> FN_PREFIX
> per architecture and has also #else branch for the rest. And more
> comments
> never hurt.
Agreed.
>
> Btw, see also
> https://sashiko.dev/#/patchset/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253%40suse.com
>
> for the Sashiko AI review. It also commented on this patch. Marcos, I
> guess that you will look there and I will just omit what Sashiko
> found in
> my review if I spot the same thing.
I already checked there. Maybe adding more context to the patch and
code will avoid further confusion about it. Let me add it in the v2.
Thanks for the reviews Miroslav and Joe!
>
> Miroslav
^ permalink raw reply
* Re: [PATCH 2/8] selftests: livepatch: test-kprobe: Replace true/false mod param by 1/0
From: Marcos Paulo de Souza @ 2026-03-19 14:16 UTC (permalink / raw)
To: Miroslav Benes
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <alpine.LSU.2.21.2603191401380.22987@pobox.suse.cz>
On Thu, 2026-03-19 at 14:03 +0100, Miroslav Benes wrote:
> A nit but I think that "test-kprobe: " is unnecessary noise in the
> subject
> and can be dropped. It applies to all patches in the series.
Ok, I'll drop it in the v2.
>
> On Fri, 13 Mar 2026, Marcos Paulo de Souza wrote:
>
> > Older kernels don't support true/false for boolean module
> > parameters
> > because they lack commit 0d6ea3ac94ca
> > ("lib/kstrtox.c: add "false"/"true" support to kstrtobool()").
> > Replace
> > true/false by 1/0 so the test module can be loaded on older
> > kernels.
> >
> > No functional changes.
>
> We also define a bool module parameter in
> test_modules/test_klp_callbacks_busy.c. Does it have a similar
> problem?
No, because n/N was accepted as false already on 4.12 (SLE12-SP5). I'm
not sure about older versions tough.
>
> Miroslav
^ permalink raw reply
* Re: [PATCH 3/8] selftests: livepatch: test-kprobe: Check if kprobes can work with livepatches
From: Marcos Paulo de Souza @ 2026-03-19 14:35 UTC (permalink / raw)
To: Joe Lawrence
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <abhqRTBtF1hLDmPq@redhat.com>
On Mon, 2026-03-16 at 16:38 -0400, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:34PM -0300, Marcos Paulo de Souza
> wrote:
> > Running the upstream selftests on older kernels can presente some
> > issues
> > regarding features being not present. One of such issues if the
> > missing
> > capability of having both kprobes and livepatches on the same
> > function.
> >
>
> nit picking, but slightly reworded for clarity and spelling:
>
> Running upstream selftests on older kernels can be problematic when
> features or fixes from newer versions are not present. For example,
> older kernels may lack the capability to support kprobes and
> livepatches
> on the same function simultaneously.
Much better, I'll pick your description for v2.
>
> > The support was introduced in commit 0bc11ed5ab60c
> > ("kprobes: Allow kprobes coexist with livepatch"), which means that
> > older
> > kernels may lack this change.
> >
> > The lack of this feature can be checked when a kprobe without a
> > post_handler is loaded and checking that the enabled_function's
> > file
> > shows the flag "I". A kernel with the proper support for kprobes
> > and
> > livepatches would presente the flag only when a post_handler is
>
> nit: s/presente/present
Ok.
>
> > registered.
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test-kprobe.sh | 52
> > ++++++++++++++----------
> > 1 file changed, 31 insertions(+), 21 deletions(-)
> >
> > diff --git a/tools/testing/selftests/livepatch/test-kprobe.sh
> > b/tools/testing/selftests/livepatch/test-kprobe.sh
> > index cdf31d0e51955..44cd16156dbd4 100755
> > --- a/tools/testing/selftests/livepatch/test-kprobe.sh
> > +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> > @@ -16,30 +16,19 @@ setup_config
> > # when it uses a post_handler since only one IPMODIFY maybe be
> > registered
> > # to any given function at a time.
> >
> > -start_test "livepatch interaction with kprobed function with
> > post_handler"
> > -
> > -echo 1 > "$SYSFS_KPROBES_DIR/enabled"
> > -
> > -load_mod $MOD_KPROBE has_post_handler=1
> > -load_failing_mod $MOD_LIVEPATCH
> > -unload_mod $MOD_KPROBE
> > -
> > -check_result "% insmod test_modules/test_klp_kprobe.ko
> > has_post_handler=1
> > -% insmod test_modules/$MOD_LIVEPATCH.ko
> > -livepatch: enabling patch '$MOD_LIVEPATCH'
> > -livepatch: '$MOD_LIVEPATCH': initializing patching transition
> > -livepatch: failed to register ftrace handler for function
> > 'cmdline_proc_show' (-16)
> > -livepatch: failed to patch object 'vmlinux'
> > -livepatch: failed to enable patch '$MOD_LIVEPATCH'
> > -livepatch: '$MOD_LIVEPATCH': canceling patching transition, going
> > to unpatch
> > -livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> > -livepatch: '$MOD_LIVEPATCH': unpatching complete
> > -insmod: ERROR: could not insert module
> > test_modules/$MOD_LIVEPATCH.ko: Device or resource busy
> > -% rmmod test_klp_kprobe"
> > -
> > start_test "livepatch interaction with kprobed function without
> > post_handler"
> >
> > load_mod $MOD_KPROBE has_post_handler=0
> > +
> > +# Check if commit 0bc11ed5ab60c ("kprobes: Allow kprobes coexist
> > with livepatch")
> > +# is missing, meaning that livepatches and kprobes can't be used
> > together.
> > +# When the commit is missing, kprobes always set IPMODIFY (the I
> > flag), even
> > +# when the post handler is missing.
> > +if grep --quiet ") R I"
> > "$SYSFS_DEBUG_DIR/tracing/enabled_functions"; then
>
> Will flags R I always be in this order?
seq_printf(m, " (%ld)%s%s%s%s%s",
ftrace_rec_count(rec),
rec->flags & FTRACE_FL_REGS ? " R" : " ",
rec->flags & FTRACE_FL_IPMODIFY ? " I" : "
",
So this is safe. I'll add a comment in the patch to explain why this is
safe too. Thanks for the comment!
>
> --
> Joe
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Miroslav Benes @ 2026-03-20 10:45 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Joe Lawrence, Josh Poimboeuf, Jiri Kosina, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <0d85d8d7533a7a78d1f8fcc1fff8ffc73b1cf225.camel@suse.com>
> > So I would perhaps prefer to stay with the logic that defines
> > FN_PREFIX
> > per architecture and has also #else branch for the rest. And more
> > comments
> > never hurt.
>
> Agreed.
Hm, so I thought about a bit more and I very likely misunderstood the
motivation behind the patch. I will speculate and correct me if I am
wrong, please. The idea behind the whole patch set is to make the
selftests run on older kernels which I think is something we should
support. The issue is that old kernels (like mentioned 4.12) do not have
syscall wrappers at all. getpid() syscall is just plain old sys_getpid
there and not the current __x64_sys_getpid on x86. The patch fixes it
by checking CONFIG_ARCH_HAS_SYSCALL_WRAPPER and defining FN_PREFIX
accordingly.
So, if this is correct, I think it should be done differently. We should
have something like syscall_wrapper.h which would define FN_PREFIX for
the supported architectures and different kernel versions since the
wrappers may have changed a couple of times during the history. In that
case there could then be an #else branch which might just error out with
the message to add proper syscall wrapper naming.
The changelog then should explain it because it is not in fact tight to
powerpc.
What do you think? Am I off again?
Miroslav
^ permalink raw reply
* Re: [PATCH 2/8] selftests: livepatch: test-kprobe: Replace true/false mod param by 1/0
From: Miroslav Benes @ 2026-03-20 11:18 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <a57eb2eb73eb8bd817196b8505ab59d5c3bc187b.camel@suse.com>
On Thu, 19 Mar 2026, Marcos Paulo de Souza wrote:
> On Thu, 2026-03-19 at 14:03 +0100, Miroslav Benes wrote:
> > A nit but I think that "test-kprobe: " is unnecessary noise in the
> > subject
> > and can be dropped. It applies to all patches in the series.
>
> Ok, I'll drop it in the v2.
>
> >
> > On Fri, 13 Mar 2026, Marcos Paulo de Souza wrote:
> >
> > > Older kernels don't support true/false for boolean module
> > > parameters
> > > because they lack commit 0d6ea3ac94ca
> > > ("lib/kstrtox.c: add "false"/"true" support to kstrtobool()").
> > > Replace
> > > true/false by 1/0 so the test module can be loaded on older
> > > kernels.
> > >
> > > No functional changes.
> >
> > We also define a bool module parameter in
> > test_modules/test_klp_callbacks_busy.c. Does it have a similar
> > problem?
>
> No, because n/N was accepted as false already on 4.12 (SLE12-SP5). I'm
> not sure about older versions tough.
strtobool() (predecessor of kstrtobool()) has it from the beginning and
that is ~2010 which predates the kernel live patching itself so I think we
are good.
Miroslav
^ permalink raw reply
* Re: [PATCH 3/8] selftests: livepatch: test-kprobe: Check if kprobes can work with livepatches
From: Petr Mladek @ 2026-03-20 11:33 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Joe Lawrence, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <c4249fb8b36aba8649e4dcdac022f2d646413756.camel@suse.com>
On Thu 2026-03-19 11:35:16, Marcos Paulo de Souza wrote:
> On Mon, 2026-03-16 at 16:38 -0400, Joe Lawrence wrote:
> > On Fri, Mar 13, 2026 at 05:58:34PM -0300, Marcos Paulo de Souza
> > wrote:
> > > Running the upstream selftests on older kernels can presente some
> > > issues
> > > regarding features being not present. One of such issues if the
> > > missing
> > > capability of having both kprobes and livepatches on the same
> > > function.
> > >
> >
> > nit picking, but slightly reworded for clarity and spelling:
> >
> > Running upstream selftests on older kernels can be problematic when
> > features or fixes from newer versions are not present. For example,
> > older kernels may lack the capability to support kprobes and
> > livepatches
> > on the same function simultaneously.
>
> Much better, I'll pick your description for v2.
>
> >
> > > The support was introduced in commit 0bc11ed5ab60c
> > > ("kprobes: Allow kprobes coexist with livepatch"), which means that
> > > older
> > > kernels may lack this change.
> > >
> > > The lack of this feature can be checked when a kprobe without a
> > > post_handler is loaded and checking that the enabled_function's
> > > file
> > > shows the flag "I". A kernel with the proper support for kprobes
> > > and
> > > livepatches would presente the flag only when a post_handler is
> >
> > nit: s/presente/present
>
> > > --- a/tools/testing/selftests/livepatch/test-kprobe.sh
> > > +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> > > @@ -16,30 +16,19 @@ setup_config
> > > # when it uses a post_handler since only one IPMODIFY maybe be
> > > registered
> > > # to any given function at a time.
> > >
> > > -start_test "livepatch interaction with kprobed function with
> > > post_handler"
> > > -
> > > -echo 1 > "$SYSFS_KPROBES_DIR/enabled"
> > > -
> > > -load_mod $MOD_KPROBE has_post_handler=1
> > > -load_failing_mod $MOD_LIVEPATCH
> > > -unload_mod $MOD_KPROBE
> > > -
> > > -check_result "% insmod test_modules/test_klp_kprobe.ko
> > > has_post_handler=1
> > > -% insmod test_modules/$MOD_LIVEPATCH.ko
> > > -livepatch: enabling patch '$MOD_LIVEPATCH'
> > > -livepatch: '$MOD_LIVEPATCH': initializing patching transition
> > > -livepatch: failed to register ftrace handler for function
> > > 'cmdline_proc_show' (-16)
> > > -livepatch: failed to patch object 'vmlinux'
> > > -livepatch: failed to enable patch '$MOD_LIVEPATCH'
> > > -livepatch: '$MOD_LIVEPATCH': canceling patching transition, going
> > > to unpatch
> > > -livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> > > -livepatch: '$MOD_LIVEPATCH': unpatching complete
> > > -insmod: ERROR: could not insert module
> > > test_modules/$MOD_LIVEPATCH.ko: Device or resource busy
> > > -% rmmod test_klp_kprobe"
> > > -
> > > start_test "livepatch interaction with kprobed function without
> > > post_handler"
> > >
> > > load_mod $MOD_KPROBE has_post_handler=0
> > > +
> > > +# Check if commit 0bc11ed5ab60c ("kprobes: Allow kprobes coexist
> > > with livepatch")
> > > +# is missing, meaning that livepatches and kprobes can't be used
> > > together.
> > > +# When the commit is missing, kprobes always set IPMODIFY (the I
> > > flag), even
> > > +# when the post handler is missing.
> > > +if grep --quiet ") R I"
> > > "$SYSFS_DEBUG_DIR/tracing/enabled_functions"; then
> >
> > Will flags R I always be in this order?
>
> seq_printf(m, " (%ld)%s%s%s%s%s",
> ftrace_rec_count(rec),
> rec->flags & FTRACE_FL_REGS ? " R" : " ",
> rec->flags & FTRACE_FL_IPMODIFY ? " I" : "
> ",
>
> So this is safe. I'll add a comment in the patch to explain why this is
> safe too. Thanks for the comment!
I would personally check also "cmdline_proc_show" to make sure that
the line is about this function. Something like:
grep --quiet ") "cmdline_proc_show.*([0-9]\+) R"
But I am afraid that this approach is not good. It breaks the test.
It won't longer be able to catch regressions when the kprobe
sets "FTRACE_FL_IPMODIFY" by mistake again.
We could add a version check. But it would break users who backport
the fix into older kernels.
IMHO, the best solution would be to keep the test as is.
Whoever is running the test with older kernels should mark it
as "failure-expected". The test is pointing out an existing problem
in the old kernel. IMHO, it should not hide it.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 5/8] selftests: livepatch: sysfs: Split tests of replace attribute
From: Miroslav Benes @ 2026-03-20 13:03 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-5-71ac6dfb3253@suse.com>
On Fri, 13 Mar 2026, Marcos Paulo de Souza wrote:
> In order to run the selftests on older kernels, split the sysfs tests to
> another file, making it able to skip the tests when the attributes
> don't exists.
>
> No functional changes.
The functional change is that the test does not run older kernels now so I
would remove the line.
Anyway, I am not entirely happy with carving all three tests out of
test-sysfs.sh to be honest. Wouldn't it be better to just hide them under
"if check_sysfs_exists" condition and keep them here? You could make it
more compact if you check for the sysfs attribute just before checking
both permissions and values the first time it is accessed.
Miroslav
^ permalink raw reply
* Re: [PATCH 8/8] selftests: livepatch: functions.sh: Extend check for taint flag kernel message
From: Miroslav Benes @ 2026-03-20 13:04 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-8-71ac6dfb3253@suse.com>
On Fri, 13 Mar 2026, Marcos Paulo de Souza wrote:
> On SLE kernels there is a warning when a livepatch is disabled:
> livepatch: attempt to disable live patch test_klp_livepatch, setting
> NO_SUPPORT taint flag
>
> Extend lightly the detection of messages when a livepatch is disabled
> to cover this case as well.
s/lightly/slightly/ ?
Miroslav
^ permalink raw reply
* Re: [PATCH 5/8] selftests: livepatch: sysfs: Split tests of replace attribute
From: Petr Mladek @ 2026-03-20 13:12 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-5-71ac6dfb3253@suse.com>
On Fri 2026-03-13 17:58:36, Marcos Paulo de Souza wrote:
> In order to run the selftests on older kernels, split the sysfs tests to
> another file, making it able to skip the tests when the attributes
> don't exists.
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> tools/testing/selftests/livepatch/Makefile | 1 +
> .../selftests/livepatch/test-sysfs-replace-attr.sh | 75 ++++++++++++++++++++++
> tools/testing/selftests/livepatch/test-sysfs.sh | 48 --------------
> 3 files changed, 76 insertions(+), 48 deletions(-)
>
> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index a080eb54a215d..b95aa6e78a273 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -10,6 +10,7 @@ TEST_PROGS := \
> test-state.sh \
> test-ftrace.sh \
> test-sysfs.sh \
> + test-sysfs-replace-attr.sh \
> test-syscall.sh \
> test-kprobe.sh
>
> diff --git a/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh b/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh
> new file mode 100755
> index 0000000000000..d1051211fe320
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test-sysfs-replace-attr.sh
> @@ -0,0 +1,75 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 Song Liu <song@kernel.org>
> +
> +. $(dirname $0)/functions.sh
> +
> +MOD_LIVEPATCH=test_klp_livepatch
> +
> +setup_config
> +
> +# - load a livepatch and verifies the sysfs replace attribute exists
> +
> +start_test "check sysfs replace attribute"
> +
> +load_lp $MOD_LIVEPATCH
> +
> +check_sysfs_exists "$MOD_LIVEPATCH" "replace"
> +file_exists=$?
> +
> +disable_lp $MOD_LIVEPATCH
> +
> +unload_lp $MOD_LIVEPATCH
> +
> +if [[ "$file_exists" == "0" ]]; then
> + skip "sysfs attribute doesn't exists."
Nit: I would write "is not supported by this kernel version"
instead of "doesn't exist".
I think that it better describes the situation.
"doesn't exist" sounds more like an error description.
> +fi
This extra code is repeated in 6th and 7th patch as well.
It would be nice to hide it into some helper function
so that we could use here something like:
check_sysfs_attr_or_skip "replace"
or
if has_klp_sysfs_attr "replace" ; then
skip "sysfs \"replace\" attribute is not supported"
fi
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 8/8] selftests: livepatch: functions.sh: Extend check for taint flag kernel message
From: Petr Mladek @ 2026-03-20 13:26 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-8-71ac6dfb3253@suse.com>
On Fri 2026-03-13 17:58:39, Marcos Paulo de Souza wrote:
> On SLE kernels there is a warning when a livepatch is disabled:
> livepatch: attempt to disable live patch test_klp_livepatch, setting
> NO_SUPPORT taint flag
>
> Extend lightly the detection of messages when a livepatch is disabled
> to cover this case as well.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> tools/testing/selftests/livepatch/functions.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index 781346d6e94e0..73a1d4e6acaeb 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
> @@ -324,7 +324,7 @@ function check_result {
> # - filter out dmesg timestamp prefixes
> result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == last_dmesg { p=1 }' | \
> grep -e 'livepatch:' -e 'test_klp' | \
> - grep -v '\(tainting\|taints\) kernel' | \
> + grep -v '\(tainting\|taints\|taint\) \(kernel\|flag\)' | \
> sed 's/^\[[ 0-9.]*\] //' | \
> sed 's/^\[[ ]*[CT][0-9]*\] //')
With the upstream maintainer hat on:
I am afraid that we could not take this. It is needed only because
of another out-of-tree patch. It does not describe the upstream
behavior. It might even hide problems.
We should maintain a SUSE-specific patch against the selftests
as a counter-part for the patch adding the tainting.
Or we could try to upstream the patch which adds the tainting.
Well, we might want to limit the tainting only to livepatches
with callbacks or shadow variables. IMHO, only these features
are source of potential problems.
Best Rerards,
Petr
^ permalink raw reply
* Re: [PATCH 0/8] kselftests: livepatch: Adapt tests to be executed on 4.12 kernels
From: Petr Mladek @ 2026-03-20 13:31 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253@suse.com>
On Fri 2026-03-13 17:58:31, Marcos Paulo de Souza wrote:
> These patches don't really change how the patches are run, just skip
> some tests on kernels that don't support a feature (like kprobe and
> livepatched living together) or when a livepatch sysfs attribute is
> missing.
>
> The last patch slightly adjusts check_result function to skip dmesg
> messages on SLE kernels when a livepatch is removed.
>
> These patches are based on printk/for-next branch.
>
> Please review! Thanks!
JFYI, I am suprised how good was the review made by the Sashiko AI, see
https://sashiko.dev/#/patchset/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253%40suse.com
It pointed out many problems mentioned in the human review.
IMHO, it was also able to explain them very well.
Well, it missed some higher level views.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 8/8] selftests: livepatch: functions.sh: Extend check for taint flag kernel message
From: Marcos Paulo de Souza @ 2026-03-20 13:41 UTC (permalink / raw)
To: Petr Mladek
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Joe Lawrence,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <ab1K6oR06sLm9LM_@pathway.suse.cz>
On Fri, 2026-03-20 at 14:26 +0100, Petr Mladek wrote:
> On Fri 2026-03-13 17:58:39, Marcos Paulo de Souza wrote:
> > On SLE kernels there is a warning when a livepatch is disabled:
> > livepatch: attempt to disable live patch test_klp_livepatch,
> > setting
> > NO_SUPPORT taint flag
> >
> > Extend lightly the detection of messages when a livepatch is
> > disabled
> > to cover this case as well.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/functions.sh | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/livepatch/functions.sh
> > b/tools/testing/selftests/livepatch/functions.sh
> > index 781346d6e94e0..73a1d4e6acaeb 100644
> > --- a/tools/testing/selftests/livepatch/functions.sh
> > +++ b/tools/testing/selftests/livepatch/functions.sh
> > @@ -324,7 +324,7 @@ function check_result {
> > # - filter out dmesg timestamp prefixes
> > result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 ==
> > last_dmesg { p=1 }' | \
> > grep -e 'livepatch:' -e 'test_klp' | \
> > - grep -v '\(tainting\|taints\) kernel' | \
> > + grep -v '\(tainting\|taints\|taint\)
> > \(kernel\|flag\)' | \
> > sed 's/^\[[ 0-9.]*\] //' | \
> > sed 's/^\[[ ]*[CT][0-9]*\] //')
>
> With the upstream maintainer hat on:
>
> I am afraid that we could not take this. It is needed only because
> of another out-of-tree patch. It does not describe the upstream
> behavior. It might even hide problems.
>
> We should maintain a SUSE-specific patch against the selftests
> as a counter-part for the patch adding the tainting.
>
> Or we could try to upstream the patch which adds the tainting.
> Well, we might want to limit the tainting only to livepatches
> with callbacks or shadow variables. IMHO, only these features
> are source of potential problems.
TBH I wasn't expecting this patch to be merged, but I send it anyway
since the change wasn't big. But I agree, we can drop this one from the
patchset.
>
> Best Rerards,
> Petr
^ permalink raw reply
* [PATCH] selftests/livepatch: add test for module function patching
From: Pablo Hugen @ 2026-03-20 20:11 UTC (permalink / raw)
To: live-patching, linux-kselftest, linux-kernel
Cc: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, shuah,
Pablo Alessandro Santos Hugen
From: Pablo Alessandro Santos Hugen <phugen@redhat.com>
Add a target module and livepatch pair that verify module function
patching via a proc entry. Two test cases cover both the
klp_enable_patch path (target loaded before livepatch) and the
klp_module_coming path (livepatch loaded before target).
Signed-off-by: Pablo Alessandro Santos Hugen <phugen@redhat.com>
---
.../selftests/livepatch/test-livepatch.sh | 100 ++++++++++++++++++
.../selftests/livepatch/test_modules/Makefile | 2 +
.../test_modules/test_klp_mod_patch.c | 53 ++++++++++
.../test_modules/test_klp_mod_target.c | 39 +++++++
4 files changed, 194 insertions(+)
create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index 6673023d2b66..c44c5341a2f1 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -8,6 +8,8 @@ MOD_LIVEPATCH1=test_klp_livepatch
MOD_LIVEPATCH2=test_klp_syscall
MOD_LIVEPATCH3=test_klp_callbacks_demo
MOD_REPLACE=test_klp_atomic_replace
+MOD_TARGET=test_klp_mod_target
+MOD_TARGET_PATCH=test_klp_mod_patch
setup_config
@@ -196,4 +198,102 @@ 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
+# - verify the proc entry returns livepatched output
+# - disable and unload the livepatch
+# - verify the proc entry returns original output again
+# - unload the target module
+
+start_test "module function patching"
+
+load_mod $MOD_TARGET
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+load_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+disable_lp $MOD_TARGET_PATCH
+unload_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+unload_mod $MOD_TARGET
+
+check_result "% insmod test_modules/$MOD_TARGET.ko
+$MOD_TARGET: test_klp_mod_target_init
+% insmod test_modules/$MOD_TARGET_PATCH.ko
+livepatch: enabling patch '$MOD_TARGET_PATCH'
+livepatch: '$MOD_TARGET_PATCH': initializing patching transition
+livepatch: '$MOD_TARGET_PATCH': starting patching transition
+livepatch: '$MOD_TARGET_PATCH': completing patching transition
+livepatch: '$MOD_TARGET_PATCH': patching complete
+% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
+livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
+livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': unpatching complete
+% rmmod $MOD_TARGET_PATCH
+% rmmod $MOD_TARGET
+$MOD_TARGET: test_klp_mod_target_exit"
+
+
+# - load a livepatch that targets a not-yet-loaded module
+# - load the target module: klp_module_coming patches it immediately
+# - verify the proc entry returns livepatched output
+# - disable and unload the livepatch
+# - verify the proc entry returns original output again
+# - unload the target module
+
+start_test "module function patching (livepatch first)"
+
+load_lp $MOD_TARGET_PATCH
+load_mod $MOD_TARGET
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+disable_lp $MOD_TARGET_PATCH
+unload_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+ echo -e "FAIL\n\n"
+ die "livepatch kselftest(s) failed"
+fi
+
+unload_mod $MOD_TARGET
+
+check_result "% insmod test_modules/$MOD_TARGET_PATCH.ko
+livepatch: enabling patch '$MOD_TARGET_PATCH'
+livepatch: '$MOD_TARGET_PATCH': initializing patching transition
+livepatch: '$MOD_TARGET_PATCH': starting patching transition
+livepatch: '$MOD_TARGET_PATCH': completing patching transition
+livepatch: '$MOD_TARGET_PATCH': patching complete
+% insmod test_modules/$MOD_TARGET.ko
+livepatch: applying patch '$MOD_TARGET_PATCH' to loading module '$MOD_TARGET'
+$MOD_TARGET: test_klp_mod_target_init
+% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
+livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
+livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': unpatching complete
+% rmmod $MOD_TARGET_PATCH
+% rmmod $MOD_TARGET
+$MOD_TARGET: test_klp_mod_target_exit"
+
+
exit 0
diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
index 939230e571f5..a13d398585dc 100644
--- a/tools/testing/selftests/livepatch/test_modules/Makefile
+++ b/tools/testing/selftests/livepatch/test_modules/Makefile
@@ -8,6 +8,8 @@ obj-m += test_klp_atomic_replace.o \
test_klp_callbacks_mod.o \
test_klp_kprobe.o \
test_klp_livepatch.o \
+ test_klp_mod_patch.o \
+ test_klp_mod_target.o \
test_klp_shadow_vars.o \
test_klp_state.o \
test_klp_state2.o \
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
new file mode 100644
index 000000000000..6725b4720365
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Pablo Hugen <phugen@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include <linux/seq_file.h>
+
+static int livepatch_mod_target_show(struct seq_file *m, void *v)
+{
+ seq_printf(m, "%s: %s\n", THIS_MODULE->name,
+ "this has been live patched");
+ return 0;
+}
+
+static struct klp_func funcs[] = {
+ {
+ .old_name = "test_klp_mod_target_show",
+ .new_func = livepatch_mod_target_show,
+ },
+ {},
+};
+
+static struct klp_object objs[] = {
+ {
+ .name = "test_klp_mod_target",
+ .funcs = funcs,
+ },
+ {},
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int test_klp_mod_patch_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void test_klp_mod_patch_exit(void)
+{
+}
+
+module_init(test_klp_mod_patch_init);
+module_exit(test_klp_mod_patch_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Pablo Hugen <phugen@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: patch for module-provided function");
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
new file mode 100644
index 000000000000..9643984d2402
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Pablo Hugen <phugen@redhat.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+
+static struct proc_dir_entry *pde;
+
+static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
+{
+ seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
+ return 0;
+}
+
+static int test_klp_mod_target_init(void)
+{
+ pr_info("%s\n", __func__);
+ pde = proc_create_single("test_klp_mod_target", 0, NULL,
+ test_klp_mod_target_show);
+ if (!pde)
+ return -ENOMEM;
+ return 0;
+}
+
+static void test_klp_mod_target_exit(void)
+{
+ pr_info("%s\n", __func__);
+ proc_remove(pde);
+}
+
+module_init(test_klp_mod_target_init);
+module_exit(test_klp_mod_target_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Hugen <phugen@redhat.com>");
+MODULE_DESCRIPTION("Livepatch test: target module with proc entry");
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Petr Pavlu @ 2026-03-23 13:06 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <20260317110423.45481-1-stf_xl@wp.pl>
On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
> over the entire symtab when resolving an address. The number of symbols
> in module symtabs has grown over the years, largely due to additional
> metadata in non-standard sections, making this lookup very slow.
>
> Improve this by separating function symbols during module load, placing
> them at the beginning of the symtab, sorting them by address, and using
> binary search when resolving addresses in module text.
Doesn't considering only function symbols break the expected behavior
with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
able to see all symbols in a module? The module loader should be remain
consistent with the main kallsyms code regarding which symbols can be
looked up.
>
> This also should improve times for linear symbol name lookups, as valid
> function symbols are now located at the beginning of the symtab.
>
> The cost of sorting is small relative to module load time. In repeated
> module load tests [1], depending on .config options, this change
> increases load time between 2% and 4%. With cold caches, the difference
> is not measurable, as memory access latency dominates.
>
> The sorting theoretically could be done in compile time, but much more
> complicated as we would have to simulate kernel addresses resolution
> for symbols, and then correct relocation entries. That would be risky
> if get out of sync.
>
> The improvement can be observed when listing ftrace filter functions:
>
> root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
> 74908
>
> real 0m1.315s
> user 0m0.000s
> sys 0m1.312s
>
> After:
>
> root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
> 74911
>
> real 0m0.167s
> user 0m0.004s
> sys 0m0.175s
>
> (there are three more symbols introduced by the patch)
This looks as a reasonable improvement.
>
> For livepatch modules, the symtab layout is preserved and the existing
> linear search is used. For this case, it should be possible to keep
> the original ELF symtab instead of copying it 1:1, but that is outside
> the scope of this patch.
Livepatch modules are already handled specially by the kallsyms module
code so excluding them from this optimization is probably ok.
However, it might be worth revisiting this exception. I believe that
livepatch support requires the original symbol table for relocations to
remain usable. It might make sense to investigate whether updating the
relocation data with the adjusted symbol indexes would be sensible.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-03-24 12:53 UTC (permalink / raw)
To: Petr Pavlu
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <b6030f42-b4d2-4e52-acec-76e25c0f40db@suse.com>
Hi,
On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
> > Module symbol lookup via find_kallsyms_symbol() performs a linear scan
> > over the entire symtab when resolving an address. The number of symbols
> > in module symtabs has grown over the years, largely due to additional
> > metadata in non-standard sections, making this lookup very slow.
> >
> > Improve this by separating function symbols during module load, placing
> > them at the beginning of the symtab, sorting them by address, and using
> > binary search when resolving addresses in module text.
>
> Doesn't considering only function symbols break the expected behavior
> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
> able to see all symbols in a module? The module loader should be remain
> consistent with the main kallsyms code regarding which symbols can be
> looked up.
We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and
module symbol lookup, independent of this patch. find_kallsyms_symbol()
restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
it cannot resolve data or rodata symbols.
This appears to be acceptable in practice, most kallsyms_lookup() users are
interested in function symbols. Users relying on CONFIG_KALLSYMS_ALL=y
seems to use name-based lookups or iterate over the full symtab. Though kdb
looks like the exception: it can resolve data symbols by address in the kernel,
but not in modules. But, I think, resolving symbols by name is more common for
kdb.
To make the behavior consistent, we could either: extend find_kallsyms_symbol()
to cover data/rodata symbols (for CONFIG_KALLSYSM_ALL), or restrict
kallsyms_lookup() to text symbols and introduce a separate API for data symbols
lookup for users that really need that. I think second option is better, as
some (maybe most) users are not interested in all symbols, even if
CONFIG_KALLSYSM_ALL is set.
However, either would require substantial rework and is outside the scope
of this patch.
Regards
Stanislaw
> > This also should improve times for linear symbol name lookups, as valid
> > function symbols are now located at the beginning of the symtab.
> >
> > The cost of sorting is small relative to module load time. In repeated
> > module load tests [1], depending on .config options, this change
> > increases load time between 2% and 4%. With cold caches, the difference
> > is not measurable, as memory access latency dominates.
> >
> > The sorting theoretically could be done in compile time, but much more
> > complicated as we would have to simulate kernel addresses resolution
> > for symbols, and then correct relocation entries. That would be risky
> > if get out of sync.
> >
> > The improvement can be observed when listing ftrace filter functions:
> >
> > root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
> > 74908
> >
> > real 0m1.315s
> > user 0m0.000s
> > sys 0m1.312s
> >
> > After:
> >
> > root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
> > 74911
> >
> > real 0m0.167s
> > user 0m0.004s
> > sys 0m0.175s
> >
> > (there are three more symbols introduced by the patch)
>
> This looks as a reasonable improvement.
>
> >
> > For livepatch modules, the symtab layout is preserved and the existing
> > linear search is used. For this case, it should be possible to keep
> > the original ELF symtab instead of copying it 1:1, but that is outside
> > the scope of this patch.
>
> Livepatch modules are already handled specially by the kallsyms module
> code so excluding them from this optimization is probably ok.
>
> However, it might be worth revisiting this exception. I believe that
> livepatch support requires the original symbol table for relocations to
> remain usable. It might make sense to investigate whether updating the
> relocation data with the adjusted symbol indexes would be sensible.
>
> --
> Thanks,
> Petr
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Miroslav Benes @ 2026-03-24 14:22 UTC (permalink / raw)
To: Pablo Hugen
Cc: live-patching, linux-kselftest, linux-kernel, jpoimboe, jikos,
mbenes, pmladek, joe.lawrence, shuah
In-Reply-To: <20260320201135.1203992-1-phugen@redhat.com>
On Fri, 20 Mar 2026 17:11:17 -0300, Pablo Hugen <phugen@redhat.com> wrote:
> Add a target module and livepatch pair that verify module function
> patching via a proc entry. Two test cases cover both the
> klp_enable_patch path (target loaded before livepatch) and the
> klp_module_coming path (livepatch loaded before target).
We sort of test the same in test-callbacks.sh. Just using different
means. I think I would not mind having this as well.
Petr, Joe, what do you think?
>
>
> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> new file mode 100644
> index 000000000000..9643984d2402
> --- /dev/null
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> @@ -0,0 +1,39 @@
> [ ... skip 11 lines ... ]
> +
> +static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
> +{
> + seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
> + return 0;
> +}
A nit but is 'noinline' keyword needed here? proc_create_single() below
takes a function pointer so hopefully test_klp_mod_target_show() stays
even without it?
> +
> +static int test_klp_mod_target_init(void)
> +{
> + pr_info("%s\n", __func__);
> + pde = proc_create_single("test_klp_mod_target", 0, NULL,
> + test_klp_mod_target_show);
...here.
Otherwise it looks good to me.
--
Miroslav
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Joe Lawrence @ 2026-03-24 14:45 UTC (permalink / raw)
To: Miroslav Benes
Cc: Pablo Hugen, live-patching, linux-kselftest, linux-kernel,
jpoimboe, jikos, pmladek, shuah
In-Reply-To: <177436214729.62466.7977538958560300344.b4-review@b4>
On Tue, Mar 24, 2026 at 03:22:27PM +0100, Miroslav Benes wrote:
> On Fri, 20 Mar 2026 17:11:17 -0300, Pablo Hugen <phugen@redhat.com> wrote:
> > Add a target module and livepatch pair that verify module function
> > patching via a proc entry. Two test cases cover both the
> > klp_enable_patch path (target loaded before livepatch) and the
> > klp_module_coming path (livepatch loaded before target).
>
> We sort of test the same in test-callbacks.sh. Just using different
> means. I think I would not mind having this as well.
>
> Petr, Joe, what do you think?
>
I was *just* in the middle of replying to the patch when yours came in,
so I'll just move over here. I had noticed the same thing re:
test-callbacks.sh despite originally suggested writing this test to
Pablo (and forgot about the callbacks test module). With that, I agree
that it's a nice basic sanity check that's obvious about what it's
testing.
> >
> >
> > diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> > new file mode 100644
> > index 000000000000..9643984d2402
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> > @@ -0,0 +1,39 @@
> > [ ... skip 11 lines ... ]
> > +
> > +static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
> > +{
> > + seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
> > + return 0;
> > +}
>
> A nit but is 'noinline' keyword needed here? proc_create_single() below
> takes a function pointer so hopefully test_klp_mod_target_show() stays
> even without it?
>
No strong preference either way. I won't fault a livepatch developer
for being paranoid w/respect to the compiler :D
Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
--
Joe
^ permalink raw reply
* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Petr Pavlu @ 2026-03-24 16:00 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <20260324125304.GA15972@wp.pl>
On 3/24/26 1:53 PM, Stanislaw Gruszka wrote:
> Hi,
>
> On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
>> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
>>> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
>>> over the entire symtab when resolving an address. The number of symbols
>>> in module symtabs has grown over the years, largely due to additional
>>> metadata in non-standard sections, making this lookup very slow.
>>>
>>> Improve this by separating function symbols during module load, placing
>>> them at the beginning of the symtab, sorting them by address, and using
>>> binary search when resolving addresses in module text.
>>
>> Doesn't considering only function symbols break the expected behavior
>> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
>> able to see all symbols in a module? The module loader should be remain
>> consistent with the main kallsyms code regarding which symbols can be
>> looked up.
>
> We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and
> module symbol lookup, independent of this patch. find_kallsyms_symbol()
> restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
> it cannot resolve data or rodata symbols.
My understanding is that find_kallsyms_symbol() can identify all symbols
in a module by their addresses. However, the issue I see with
MOD_TEXT/MOD_INIT_TEXT is that the function may incorrectly calculate
the size of symbols that are not within these ranges, which is a bug
that should be fixed.
A test using kdb confirms that non-text symbols can be found by their
addresses. The following shows the current behavior with 7.0-rc5 when
printing a module parameter in mlx4_en:
[1]kdb> mds __param_arr_num_vfs
0xffffffffc1209f20 0000000100000003 ........
0xffffffffc1209f28 ffffffffc0fbf07c [mlx4_core]num_vfs_argc
0xffffffffc1209f30 ffffffff8844bba0 param_ops_byte
0xffffffffc1209f38 ffffffffc0fbf080 [mlx4_core]num_vfs
0xffffffffc1209f40 000000785f69736d msi_x...
0xffffffffc1209f48 656c5f6775626564 debug_le
0xffffffffc1209f50 00000000006c6576 vel.....
0xffffffffc1209f58 0000000000000000 ........
.. and the behavior with the proposed patch:
[1]kdb> mds __param_arr_num_vfs
0xffffffffc1077f20 0000000100000003 ........
0xffffffffc1077f28 ffffffffc104707c |p......
0xffffffffc1077f30 ffffffffb4a4bba0 param_ops_byte
0xffffffffc1077f38 ffffffffc1047080 .p......
0xffffffffc1077f40 000000785f69736d msi_x...
0xffffffffc1077f48 656c5f6775626564 debug_le
0xffffffffc1077f50 00000000006c6576 vel.....
0xffffffffc1077f58 0000000000000000 ........
--
Thanks,
Petr
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox