* [PATCH v3 4/6] selftests: livepatch: Check if patched sysfs attribute exists
From: Marcos Paulo de Souza @ 2026-04-27 18:31 UTC (permalink / raw)
To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Shuah Khan
Cc: live-patching, linux-kselftest, linux-kernel,
Marcos Paulo de Souza
In-Reply-To: <20260427-lp-tests-old-fixes-v3-0-ccf3c90f744c@suse.com>
The commit bb26cfd9e77e
("livepatch: add sysfs entry "patched" for each klp_object") was merged
in v6.1, introducing a new sysfs attribute.
In order to run the selftests on older kernels, check if given kernel
has support for the attribute. If the attribute is not supported, skip
the checks.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
tools/testing/selftests/livepatch/test-sysfs.sh | 38 +++++++++++++++----------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
index 58fe1d96997c..394cf3ff99cd 100755
--- a/tools/testing/selftests/livepatch/test-sysfs.sh
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -8,6 +8,8 @@ MOD_LIVEPATCH=test_klp_livepatch
MOD_LIVEPATCH2=test_klp_callbacks_demo
MOD_LIVEPATCH3=test_klp_syscall
+HAS_PATCH_ATTR=0
+
setup_config
# - load a livepatch and verifies the sysfs entries work as expected
@@ -25,8 +27,12 @@ check_sysfs_rights "$MOD_LIVEPATCH" "stack_order" "-r--r--r--"
check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
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--"
-check_sysfs_value "$MOD_LIVEPATCH" "vmlinux/patched" "1"
+
+if does_sysfs_exist "$MOD_LIVEPATCH/vmlinux" "patched"; then
+ check_sysfs_rights "$MOD_LIVEPATCH" "vmlinux/patched" "-r--r--r--"
+ check_sysfs_value "$MOD_LIVEPATCH" "vmlinux/patched" "1"
+ HAS_PATCH_ATTR=1
+fi
disable_lp $MOD_LIVEPATCH
@@ -45,23 +51,24 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
livepatch: '$MOD_LIVEPATCH': unpatching complete
% rmmod $MOD_LIVEPATCH"
-start_test "sysfs test object/patched"
+if [[ "$HAS_PATCH_ATTR" == "1" ]]; then
+ start_test "sysfs test object/patched"
-MOD_LIVEPATCH=test_klp_callbacks_demo
-MOD_TARGET=test_klp_callbacks_mod
-load_lp $MOD_LIVEPATCH
+ MOD_LIVEPATCH=test_klp_callbacks_demo
+ MOD_TARGET=test_klp_callbacks_mod
+ load_lp $MOD_LIVEPATCH
-# check the "patch" file changes as target module loads/unloads
-check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
-load_mod $MOD_TARGET
-check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "1"
-unload_mod $MOD_TARGET
-check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
+ # check the "patch" file changes as target module loads/unloads
+ check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
+ load_mod $MOD_TARGET
+ check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "1"
+ unload_mod $MOD_TARGET
+ check_sysfs_value "$MOD_LIVEPATCH" "$MOD_TARGET/patched" "0"
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
+ disable_lp $MOD_LIVEPATCH
+ unload_lp $MOD_LIVEPATCH
-check_result "% insmod test_modules/test_klp_callbacks_demo.ko
+ check_result "% insmod test_modules/test_klp_callbacks_demo.ko
livepatch: enabling patch 'test_klp_callbacks_demo'
livepatch: 'test_klp_callbacks_demo': initializing patching transition
test_klp_callbacks_demo: pre_patch_callback: vmlinux
@@ -87,6 +94,7 @@ livepatch: 'test_klp_callbacks_demo': completing unpatching transition
test_klp_callbacks_demo: post_unpatch_callback: vmlinux
livepatch: 'test_klp_callbacks_demo': unpatching complete
% rmmod test_klp_callbacks_demo"
+fi
start_test "sysfs test replace enabled"
--
2.54.0
^ permalink raw reply related
* [PATCH v3 3/6] selftests: livepatch: Introduce does_sysfs_exist function
From: Marcos Paulo de Souza @ 2026-04-27 18:30 UTC (permalink / raw)
To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Shuah Khan
Cc: live-patching, linux-kselftest, linux-kernel,
Marcos Paulo de Souza
In-Reply-To: <20260427-lp-tests-old-fixes-v3-0-ccf3c90f744c@suse.com>
Returns true if the livepatch sysfs attribute exists, and false otherwise.
This new function will be used in the next patches.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
tools/testing/selftests/livepatch/functions.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 8ec0cb64ad94..2bc50271729c 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -339,6 +339,16 @@ function check_result {
fi
}
+# does_sysfs_exist(modname, attr) - check sysfs attribute existence
+# modname - livepatch module creating the sysfs interface
+# attr - attribute name to be checked
+function does_sysfs_exist() {
+ local mod="$1"; shift
+ local attr="$1"; shift
+
+ [[ -f "$SYSFS_KLP_DIR/$mod/$attr" ]]
+}
+
# check_sysfs_rights(modname, rel_path, expected_rights) - check sysfs
# path permissions
# modname - livepatch module creating the sysfs interface
--
2.54.0
^ permalink raw reply related
* [PATCH v3 2/6] selftests: livepatch: Replace true/false module parameter by y/n
From: Marcos Paulo de Souza @ 2026-04-27 18:30 UTC (permalink / raw)
To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Shuah Khan
Cc: live-patching, linux-kselftest, linux-kernel,
Marcos Paulo de Souza
In-Reply-To: <20260427-lp-tests-old-fixes-v3-0-ccf3c90f744c@suse.com>
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 y/n so the test module can be loaded on older kernels.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
tools/testing/selftests/livepatch/test-kprobe.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/livepatch/test-kprobe.sh b/tools/testing/selftests/livepatch/test-kprobe.sh
index b67dfad03d97..7ced4082cff3 100755
--- a/tools/testing/selftests/livepatch/test-kprobe.sh
+++ b/tools/testing/selftests/livepatch/test-kprobe.sh
@@ -20,11 +20,11 @@ start_test "livepatch interaction with kprobed function with post_handler"
echo 1 > "$SYSFS_KPROBES_DIR/enabled"
-load_mod $MOD_KPROBE has_post_handler=true
+load_mod $MOD_KPROBE has_post_handler=y
load_failing_mod $MOD_LIVEPATCH
unload_mod $MOD_KPROBE
-check_result "% insmod test_modules/test_klp_kprobe.ko has_post_handler=true
+check_result "% insmod test_modules/test_klp_kprobe.ko has_post_handler=y
% insmod test_modules/$MOD_LIVEPATCH.ko
livepatch: enabling patch '$MOD_LIVEPATCH'
livepatch: '$MOD_LIVEPATCH': initializing patching transition
@@ -39,14 +39,14 @@ insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: Device or
start_test "livepatch interaction with kprobed function without post_handler"
-load_mod $MOD_KPROBE has_post_handler=false
+load_mod $MOD_KPROBE has_post_handler=n
load_lp $MOD_LIVEPATCH
unload_mod $MOD_KPROBE
disable_lp $MOD_LIVEPATCH
unload_lp $MOD_LIVEPATCH
-check_result "% insmod test_modules/test_klp_kprobe.ko has_post_handler=false
+check_result "% insmod test_modules/test_klp_kprobe.ko has_post_handler=n
% insmod test_modules/$MOD_LIVEPATCH.ko
livepatch: enabling patch '$MOD_LIVEPATCH'
livepatch: '$MOD_LIVEPATCH': initializing patching transition
--
2.54.0
^ permalink raw reply related
* [PATCH v3 1/6] selftests: livepatch: Check for ARCH_HAS_SYSCALL_WRAPPER config
From: Marcos Paulo de Souza @ 2026-04-27 18:30 UTC (permalink / raw)
To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Shuah Khan
Cc: live-patching, linux-kselftest, linux-kernel,
Marcos Paulo de Souza
In-Reply-To: <20260427-lp-tests-old-fixes-v3-0-ccf3c90f744c@suse.com>
Older kernels that lack CONFIG_ARCH_HAS_SYSCALL_WRAPPER config don't
have any prefixes for their syscalls. The same applies to current
powerpc and loongarch, covering all currently supported architectures
that support livepatch.
The other supported architectures have specific prefixes, so error out
when a new architecture adds livepatch support with wrappers but didn't
update the test to include it.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
.../livepatch/test_modules/test_klp_syscall.c | 27 +++++++++++++++-------
1 file changed, 19 insertions(+), 8 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 dd802783ea84..0630ffd9d9a1 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,26 @@
#include <linux/slab.h>
#include <linux/livepatch.h>
-#if defined(__x86_64__)
-#define FN_PREFIX __x64_
-#elif defined(__s390x__)
-#define FN_PREFIX __s390x_
-#elif defined(__aarch64__)
-#define FN_PREFIX __arm64_
+/*
+ * Before CONFIG_ARCH_HAS_SYSCALL_WRAPPER was introduced there were no
+ * prefixes for system calls.
+ * powerpc set this config based on configs, so it can be enabled or not.
+ */
+#if defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
+ #if defined(__x86_64__)
+ #define FN_PREFIX __x64_
+ #elif defined(__s390x__)
+ #define FN_PREFIX __s390x_
+ #elif defined(__aarch64__)
+ #define FN_PREFIX __arm64_
+ #elif defined(__powerpc__)
+ #define FN_PREFIX
+ #else
+ #error "Missing syscall wrapper for the given architecture."
+ #endif
#else
-/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
-#define FN_PREFIX
+ /* Do not set a prefix for architectures that do not enable wrappers. */
+ #define FN_PREFIX
#endif
/* Protects klp_pids */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 0/6] kselftests: livepatch: Adapt tests to be executed on 4.12 kernels
From: Marcos Paulo de Souza @ 2026-04-27 18:30 UTC (permalink / raw)
To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Shuah Khan
Cc: live-patching, linux-kselftest, linux-kernel,
Marcos Paulo de Souza
This is the third version of the patchset, now with far less changes. There
are still somethings that I would like to work next, like adapting the
newest test introduced in the last submsision, but this is something for
a new iteration.
Original cover-letter:
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.
These patches are based on printk/for-next branch.
Please review! Thanks!
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes in v3:
- Patch 1 was changed to reorganize the ifdeffery to handle multiple archs syscall wrapper (Miroslav)
- Patch 3 was changed to rework the commit message and to address function naming (Joe)
- Patches 4, 5 and 6 where had the commit messages to include the kernel version where
the given sysfs attributes were included (Petr Mladek)
- Link to v2: https://patch.msgid.link/20260413-lp-tests-old-fixes-v2-0-367c7cb5006f@suse.com
Changes in v2:
- Patch descriptions were changed to remove "test-X", since it was polluting the commit subjects (Miroslav Benes)
- Patch 8 was dropped since it was checking for a message from an out-of-tree patch. (Petr Mladek)
- Patch 3 was dropped as should be treated as expected failure for older kernels. (Petr Mladek)
- Patch 2 was changed to use y/n instead of 1/0, since it's more natural to use it.
- Patch 1 was changed to handle ppc and loongson, and error out if dealing with a different architecture that sets
CONFIG_ARCH_HAS_SYSCALL_WRAPPER and haven't changed the test to include the proper wrapper prefix.
- Patch 4 was changed to invert the return of the bash function to return 1 in failure, like
a normal bash function (Joe Lawrence)
- Patches 5, 6 an 7 were changed to not split the tests, but to only run the tests
when the attribute were present (Miroslav Benes)
- Link to v1: https://patch.msgid.link/20260313-lp-tests-old-fixes-v1-0-71ac6dfb3253@suse.com
---
Marcos Paulo de Souza (6):
selftests: livepatch: Check for ARCH_HAS_SYSCALL_WRAPPER config
selftests: livepatch: Replace true/false module parameter by y/n
selftests: livepatch: Introduce does_sysfs_exist function
selftests: livepatch: Check if patched sysfs attribute exists
selftests: livepatch: Check if replace sysfs attribute exists
selftests: livepatch: Check if stack_order sysfs attribute exists
tools/testing/selftests/livepatch/functions.sh | 10 ++
tools/testing/selftests/livepatch/test-kprobe.sh | 8 +-
tools/testing/selftests/livepatch/test-sysfs.sh | 120 ++++++++++++---------
.../livepatch/test_modules/test_klp_syscall.c | 27 +++--
4 files changed, 104 insertions(+), 61 deletions(-)
---
base-commit: b8e6ad22f78aa279dece2f86efe6429953d36452
change-id: 20260309-lp-tests-old-fixes-f955abc8ec27
Best regards,
--
Marcos Paulo de Souza <mpdesouza@suse.com>
^ permalink raw reply
* Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT
From: Josh Poimboeuf @ 2026-04-27 17:17 UTC (permalink / raw)
To: Peter Zijlstra
Cc: x86, linux-kernel, live-patching, Joe Lawrence, Song Liu,
Miroslav Benes, Petr Mladek
In-Reply-To: <20260424094530.GD3126523@noisy.programming.kicks-ass.net>
On Fri, Apr 24, 2026 at 11:45:30AM +0200, Peter Zijlstra wrote:
> On Thu, Apr 23, 2026 at 08:38:02PM -0700, Josh Poimboeuf wrote:
>
> > I discovered it's not just FineIBT, it's basically any CALL_PADDING+CFI,
> > like so:
>
> Indeed. This looks good, thanks!
That was unncessarily creating .cfi_sites for the CFI+CALL_THUNKS case,
folding in the below:
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index d8ce7ad8c2c9..7f803796d20c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,8 +200,9 @@ objtool-cmds-$(CONFIG_HAVE_UACCESS_VALIDATION) += --uaccess
objtool-cmds-y += $(OBJTOOL_ARGS)
# objtool options
-ifdef CONFIG_CFI
-objtool-opts-$(CONFIG_CALL_PADDING) += --cfi
+ifdef CONFIG_CALL_PADDING
+objtool-opts-$(CONFIG_CFI) += --cfi
+objtool-opts-$(CONFIG_FINEIBT) += --fineibt
endif
ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
objtool-opts-$(CONFIG_HAVE_OBJTOOL_NOP_MCOUNT) += --mnop
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 790de0cb445d..bd84f5b7c9ee 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -91,7 +91,8 @@ static const struct option check_options[] = {
OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
OPT_GROUP("Options:"),
- OPT_BOOLEAN(0, "cfi", &opts.cfi, "annotate and grow kCFI preamble symbols (use with --prefix)"),
+ OPT_BOOLEAN(0, "cfi", &opts.cfi, "grow kCFI preamble symbols (use with --prefix)"),
+ OPT_BOOLEAN(0, "fineibt", &opts.fineibt, "create .cfi_sites section for FineIBT"),
OPT_BOOLEAN(0, "backtrace", &opts.backtrace, "unwind on error"),
OPT_BOOLEAN(0, "backup", &opts.backup, "create backup (.orig) file on warning/error"),
OPT_BOOLEAN(0, "dry-run", &opts.dryrun, "don't write modifications"),
@@ -163,6 +164,11 @@ static bool opts_valid(void)
return false;
}
+ if (opts.fineibt && !opts.cfi) {
+ ERROR("--fineibt requires --cfi");
+ return false;
+ }
+
if (opts.disas ||
opts.hack_jump_label ||
opts.hack_noinstr ||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 11dad0d0b6ae..ac0a48145bf7 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -912,6 +912,29 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)
return 0;
}
+static int grow_cfi_symbols(struct objtool_file *file)
+{
+ struct symbol *sym;
+
+ for_each_sym(file->elf, sym) {
+ if (!is_func_sym(sym) || !strstarts(sym->name, "__cfi_"))
+ continue;
+
+ /*
+ * Grow the __cfi_ symbol to fill the NOP gap between the
+ * 'mov <hash>, %rax' and the start of the function.
+ */
+ if (sym->len == 5) {
+ sym->len += opts.prefix;
+ sym->sym.st_size = sym->len;
+ if (elf_write_symbol(file->elf, sym))
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int create_cfi_sections(struct objtool_file *file)
{
struct section *sec;
@@ -954,17 +977,6 @@ static int create_cfi_sections(struct objtool_file *file)
return -1;
idx++;
-
- /*
- * Grow the __cfi_ symbol to fill the NOP gap between the
- * 'mov <hash>, %rax' and the start of the function.
- */
- if (sym->len == 5) {
- sym->len += opts.prefix;
- sym->sym.st_size = sym->len;
- if (elf_write_symbol(file->elf, sym))
- return -1;
- }
}
return 0;
@@ -4885,12 +4897,6 @@ int check(struct objtool_file *file)
goto out;
}
- if (opts.cfi) {
- ret = create_cfi_sections(file);
- if (ret)
- goto out;
- }
-
if (opts.rethunk) {
ret = create_return_sites_sections(file);
if (ret)
@@ -4909,10 +4915,22 @@ int check(struct objtool_file *file)
goto out;
}
- if (opts.prefix && !opts.cfi) {
- ret = create_prefix_symbols(file);
- if (ret)
- goto out;
+ if (opts.prefix) {
+ if (!opts.cfi) {
+ ret = create_prefix_symbols(file);
+ if (ret)
+ goto out;
+ } else {
+ ret = grow_cfi_symbols(file);
+ if (ret)
+ goto out;
+
+ if (opts.fineibt) {
+ ret = create_cfi_sections(file);
+ if (ret)
+ goto out;
+ }
+ }
}
if (opts.ibt) {
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index b9e229ed4dc0..e844e9c82b7b 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -9,8 +9,8 @@
struct opts {
/* actions: */
- bool cfi;
bool checksum;
+ const char *disas;
bool dump_orc;
bool hack_jump_label;
bool hack_noinstr;
@@ -20,6 +20,7 @@ struct opts {
bool noabs;
bool noinstr;
bool orc;
+ int prefix;
bool retpoline;
bool rethunk;
bool unret;
@@ -27,14 +28,14 @@ struct opts {
bool stackval;
bool static_call;
bool uaccess;
- int prefix;
- const char *disas;
/* options: */
bool backtrace;
bool backup;
+ bool cfi;
const char *debug_checksum;
bool dryrun;
+ bool fineibt;
bool link;
bool mnop;
bool module;
^ permalink raw reply related
* Re: [PATCH v2 2/2] module/kallsyms: sort function symbols and use binary search
From: Petr Pavlu @ 2026-04-27 13:51 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: <20260424091330.GA31168@wp.pl>
On 4/24/26 11:13 AM, Stanislaw Gruszka wrote:
> On Thu, Apr 23, 2026 at 04:00:04PM +0200, Petr Pavlu wrote:
>> On 3/27/26 12:00 PM, Stanislaw Gruszka wrote:
[...]
>>> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
>>> index f23126d804b2..d69e99e67707 100644
>>> --- a/kernel/module/kallsyms.c
>>> +++ b/kernel/module/kallsyms.c
>>> @@ -10,6 +10,7 @@
>>> #include <linux/kallsyms.h>
>>> #include <linux/buildid.h>
>>> #include <linux/bsearch.h>
>>> +#include <linux/sort.h>
>>> #include "internal.h"
>>>
>>> /* Lookup exported symbol in given range of kernel_symbols */
>>> @@ -103,6 +104,95 @@ static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
>>> return true;
>>> }
>>>
>>> +static inline bool is_func_symbol(const Elf_Sym *sym)
>>> +{
>>> + return sym->st_shndx != SHN_UNDEF && sym->st_size != 0 &&
>>> + ELF_ST_TYPE(sym->st_info) == STT_FUNC;
>>> +}
>>> +
>>> +static unsigned int bsearch_func_symbol(struct mod_kallsyms *kallsyms,
>>> + unsigned long addr,
>>> + unsigned long *bestval,
>>> + unsigned long *nextval)
>>> +
>>> +{
>>> + unsigned int mid, low = 1, high = kallsyms->num_func_syms + 1;
>>> + unsigned int best = 0;
>>> + unsigned long thisval;
>>> +
>>> + while (low < high) {
>>> + mid = low + (high - low) / 2;
>>> + thisval = kallsyms_symbol_value(&kallsyms->symtab[mid]);
>>> +
>>> + if (thisval <= addr) {
>>> + *bestval = thisval;
>>> + best = mid;
>>> + low = mid + 1;
>>
>> If thisval == addr, the search moves to the right and finds the last
>> symbol with the same address. I believe it should do the opposite and
>> return the first symbol to match the behavior of
>> search_kallsyms_symbol().
>
> In the case of multiple symbols sharing the same address, we have
> to pick one and ignore the others. I don’t think it matters much which
> one is chosen in practice. Also, I expect function symbol addresses
> to be unique, so this shouldn’t be a real issue.
I think that the code should consistently pick the same answer. If
someone uses aliases for their functions, the logic shouldn't
arbitrarily return one of them, but preferably the first one, which
should normally be the actual implementation.
>
>>> + } else {
>>> + *nextval = thisval;
>>> + high = mid;
>>> + }
>>> + }
>>> +
>>> + return best;
>>> +}
>>> +
>>> +static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms,
>>> + unsigned int symnum)
>>> +{
>>> + return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
>>> +}
>>> +
>>> +static unsigned int search_kallsyms_symbol(struct mod_kallsyms *kallsyms,
>>> + unsigned long addr,
>>> + unsigned long *bestval,
>>> + unsigned long *nextval)
>>> +{
>>> + unsigned int i, best = 0;
>>> +
>>> + /*
>>> + * Scan for closest preceding symbol and next symbol. (ELF starts
>>> + * real symbols at 1). Skip the initial function symbols range
>>> + * if num_func_syms is non-zero, those are handled separately for
>>> + * the core TEXT segment lookup.
>>> + */
>>> + for (i = 1 + kallsyms->num_func_syms; i < kallsyms->num_symtab; i++) {
>>> + const Elf_Sym *sym = &kallsyms->symtab[i];
>>> + unsigned long thisval = kallsyms_symbol_value(sym);
>>> +
>>> + if (sym->st_shndx == SHN_UNDEF)
>>> + continue;
>>> +
>>> + /*
>>> + * We ignore unnamed symbols: they're uninformative
>>> + * and inserted at a whim.
>>> + */
>>> + if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
>>> + is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
>>> + continue;
>>> +
>>> + if (thisval <= addr && thisval > *bestval) {
>>> + best = i;
>>> + *bestval = thisval;
>>> + }
>>> + if (thisval > addr && thisval < *nextval)
>>> + *nextval = thisval;
>>> + }
>>> +
>>> + return best;
>>> +}
>>> +
>>> +static int elf_sym_cmp(const void *a, const void *b)
>>> +{
>>> + unsigned long val_a = kallsyms_symbol_value((const Elf_Sym *)a);
>>> + unsigned long val_b = kallsyms_symbol_value((const Elf_Sym *)b);
>>> +
>>> + if (val_a < val_b)
>>> + return -1;
>>> +
>>> + return val_a > val_b;
>>
>> Does this comparison function and the sort() call result in stable
>> sorting? If val_a and val_b are the same, the sorting should preserve
>> the original order.
>
> The kernel’s sort() implementation is not stable.
Ok, I see it is a heapsort. It would require additional data to keep
information about the original indexes for elf_sym_cmp() to use as
a tiebreaker.
>
>>> +}
>>> +
>>> /*
>>> * We only allocate and copy the strings needed by the parts of symtab
>>> * we keep. This is simple, but has the effect of making multiple
>>> @@ -115,9 +205,10 @@ void layout_symtab(struct module *mod, struct load_info *info)
>>> Elf_Shdr *symsect = info->sechdrs + info->index.sym;
>>> Elf_Shdr *strsect = info->sechdrs + info->index.str;
>>> const Elf_Sym *src;
>>> - unsigned int i, nsrc, ndst, strtab_size = 0;
>>> + unsigned int i, nsrc, ndst, nfunc, strtab_size = 0;
>>> struct module_memory *mod_mem_data = &mod->mem[MOD_DATA];
>>> struct module_memory *mod_mem_init_data = &mod->mem[MOD_INIT_DATA];
>>> + bool is_lp_mod = is_livepatch_module(mod);
>>>
>>> /* Put symbol section at end of init part of module. */
>>> symsect->sh_flags |= SHF_ALLOC;
>>> @@ -129,12 +220,14 @@ void layout_symtab(struct module *mod, struct load_info *info)
>>> nsrc = symsect->sh_size / sizeof(*src);
>>>
>>> /* Compute total space required for the core symbols' strtab. */
>>> - for (ndst = i = 0; i < nsrc; i++) {
>>> - if (i == 0 || is_livepatch_module(mod) ||
>>> + for (ndst = nfunc = i = 0; i < nsrc; i++) {
>>> + if (i == 0 || is_lp_mod ||
>>> is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
>>> info->index.pcpu)) {
>>> strtab_size += strlen(&info->strtab[src[i].st_name]) + 1;
>>> ndst++;
>>> + if (!is_lp_mod && is_func_symbol(src + i))
>>> + nfunc++;
>>> }
>>> }
>>>
>>> @@ -156,6 +249,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
>>> mod_mem_init_data->size = ALIGN(mod_mem_init_data->size,
>>> __alignof__(struct mod_kallsyms));
>>> info->mod_kallsyms_init_off = mod_mem_init_data->size;
>>> + info->num_func_syms = nfunc;
>>>
>>> mod_mem_init_data->size += sizeof(struct mod_kallsyms);
>>> info->init_typeoffs = mod_mem_init_data->size;
>>> @@ -169,7 +263,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
>>> */
>>> void add_kallsyms(struct module *mod, const struct load_info *info)
>>> {
>>> - unsigned int i, ndst;
>>> + unsigned int i, di, nfunc, ndst;
>>> const Elf_Sym *src;
>>> Elf_Sym *dst;
>>> char *s;
>>> @@ -178,6 +272,7 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
>>> void *data_base = mod->mem[MOD_DATA].base;
>>> void *init_data_base = mod->mem[MOD_INIT_DATA].base;
>>> struct mod_kallsyms *kallsyms;
>>> + bool is_lp_mod = is_livepatch_module(mod);
>>>
>>> kallsyms = init_data_base + info->mod_kallsyms_init_off;
>>
>> This code is followed by the initialization of kallsyms:
>>
>> kallsyms->symtab = (void *)symsec->sh_addr;
>> kallsyms->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
>> /* Make sure we get permanent strtab: don't use info->strtab. */
>> kallsyms->strtab = (void *)info->sechdrs[info->index.str].sh_addr;
>> kallsyms->typetab = init_data_base + info->init_typeoffs;
>>
>> I suggest adding 'kallsyms->num_func_syms = 0;' after the initialization
>> of kallsyms->num_symtab.
>
> I relied on zeroed memory initialization, but I can add this explicitly
> for clarity.
>
>>> @@ -194,19 +289,28 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
>>> mod->core_kallsyms.symtab = dst = data_base + info->symoffs;
>>> mod->core_kallsyms.strtab = s = data_base + info->stroffs;
>>> mod->core_kallsyms.typetab = data_base + info->core_typeoffs;
>>> +
>>> strtab_size = info->core_typeoffs - info->stroffs;
>>> src = kallsyms->symtab;
>>> - for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
>>> + ndst = info->num_func_syms + 1;
>>> +
>>> + for (nfunc = i = 0; i < kallsyms->num_symtab; i++) {
>>> kallsyms->typetab[i] = elf_type(src + i, info);
>>> - if (i == 0 || is_livepatch_module(mod) ||
>>> + if (i == 0 || is_lp_mod ||
>>> is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
>>> info->index.pcpu)) {
>>> ssize_t ret;
>>>
>>> - mod->core_kallsyms.typetab[ndst] =
>>> - kallsyms->typetab[i];
>>> - dst[ndst] = src[i];
>>> - dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
>>> + if (i == 0)
>>> + di = 0;
>>> + else if (!is_lp_mod && is_func_symbol(src + i))
>>> + di = 1 + nfunc++;
>>> + else
>>> + di = ndst++;
>>> +
>>> + mod->core_kallsyms.typetab[di] = kallsyms->typetab[i];
>>> + dst[di] = src[i];
>>> + dst[di].st_name = s - mod->core_kallsyms.strtab;
>>> ret = strscpy(s, &kallsyms->strtab[src[i].st_name],
>>> strtab_size);
>>> if (ret < 0)
>>> @@ -216,9 +320,13 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
>>> }
>>> }
>>>
>>> + WARN_ON_ONCE(nfunc != info->num_func_syms);
>>> + sort(dst + 1, nfunc, sizeof(Elf_Sym), elf_sym_cmp, NULL);
>>> +
>>
>> The code sorts mod->core_kallsyms.symtab but mod->core_kallsyms.typetab
>> is not reordered accordingly.
>
> Right, but for function symbols the typetab entries are all 't',
> so swapping them does not change the type value. The 'T' vs 't'
> distinction is handled later when printing (based on export status).
> But the comment explaining skiping adjusting of
> mod->core_kallsyms.typetab is needed.
Modules can also contain weak functions with elf_type() = 'w'.
>
>>> /* Set up to point into init section. */
>>> rcu_assign_pointer(mod->kallsyms, kallsyms);
>>> mod->core_kallsyms.num_symtab = ndst;
>>> + mod->core_kallsyms.num_func_syms = nfunc;
>>> }
>>>
>>> #if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
>>> @@ -241,11 +349,6 @@ void init_build_id(struct module *mod, const struct load_info *info)
>>> }
>>> #endif
>>>
>>> -static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned int symnum)
>>> -{
>>> - return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
>>> -}
>>> -
>>> /*
>>> * Given a module and address, find the corresponding symbol and return its name
>>> * while providing its size and offset if needed.
>>> @@ -255,7 +358,10 @@ static const char *find_kallsyms_symbol(struct module *mod,
>>> unsigned long *size,
>>> unsigned long *offset)
>>> {
>>> - unsigned int i, best = 0;
>>> + unsigned int (*search)(struct mod_kallsyms *kallsyms,
>>> + unsigned long addr, unsigned long *bestval,
>>> + unsigned long *nextval);
>>> + unsigned int best;
>>> unsigned long nextval, bestval;
>>> struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
>>> struct module_memory *mod_mem = NULL;
>>> @@ -266,6 +372,11 @@ static const char *find_kallsyms_symbol(struct module *mod,
>>> continue;
>>> #endif
>>> if (within_module_mem_type(addr, mod, type)) {
>>> + if (type == MOD_TEXT && kallsyms->num_func_syms > 0)
>>> + search = bsearch_func_symbol;
>>
>> I'm not sure if it is ok to limit the search only to function symbols
>> when the address lies in MOD_TEXT. The text can theoretically contain
>> non-function symbols.
>
> Yes, the patch assumes that the only valid symbols in the MOD_TEXT
> are functions. If there are defined OBJECT symbols in .text, the patch
> would break lookup for those.
>
> While it’s theoretically possible (e.g. hand-written assembly placing
> data in .text ?), I’m not sure this is a practical concern. In general,
> having data in executable segments is discouraged for security reasons.
>
>> Could this optimization be adjusted to sort all
>> MOD_TEXT symbols (excluding anonymous and mapping symbols) and move them
>> to the front of the symbol table?
>
> That’s possible. We could track .text sections indices in
> __layout_sections() and include all valid symbols from those sections,
> and also reorder typetab accordingly.
>
> However, this adds complexity. I would prefer to first confirm whether
> OBJECT symbols in MOD_TEXT is a real issue before going in that direction.
I'm not aware of specific OBJECT symbols that end up in MOD_TEXT.
Nonetheless, it is a valid case and it is preferable that an
optimization doesn't break their lookup by address.
In general, I'm worried about the several edge cases and inconsistencies
that this optimization introduces. This also includes the fact that it
doesn't work for livepatch modules.
An alternative could be to keep the symbol table untouched and have
a separate array with symbol indexes that is sorted by their addresses,
but it requires evaluation if the additional memory usage is worth it.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4 7/8] sframe: Introduce in-kernel SFRAME_VALIDATION
From: Dylan Hatch @ 2026-04-27 7:03 UTC (permalink / raw)
To: Jens Remus
Cc: Roman Gushchin, Weinan Liu, Will Deacon, Josh Poimboeuf,
Indu Bhagat, Peter Zijlstra, Steven Rostedt, Catalin Marinas,
Jiri Kosina, Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan,
Song Liu, joe.lawrence, linux-toolchains, linux-kernel,
live-patching, linux-arm-kernel, Randy Dunlap, Heiko Carstens
In-Reply-To: <73e99161-c246-467d-96c2-46911ffc0bff@linux.ibm.com>
On Wed, Apr 22, 2026 at 7:11 AM Jens Remus <jremus@linux.ibm.com> wrote:
>
> On 4/22/2026 12:51 AM, Dylan Hatch wrote:
> > Generalize the __safe* helpers to support a non-user-access code path.
> >
> > This requires arch-specific function address validation. This is because
> > arm64 vmlinux has an .rodata.text section which lies outside the bounds
> > of the normal .text. It contains code that is never executed by the
> > kernel mapping, but for which the toolchain nonetheless generates sframe
> > data, and needs to be considered valid for a PC lookup.
> >
> > This arch-specific address validation logic is only necessary to support
> > SFRAME_VALIDATION for the vmlinux .sframe, since these .rodata.text
> > functions would never be encountered during normal unwinding.
> >
> > Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> > Suggested-by: Jens Remus <jremus@linux.ibm.com>
>
> With the minor nit below fixed:
>
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
>
> > ---
> > arch/Kconfig | 2 +-
> > arch/arm64/include/asm/sections.h | 1 +
> > arch/arm64/include/asm/unwind_sframe.h | 21 +++++++++++++++++++++
> > arch/arm64/kernel/vmlinux.lds.S | 2 ++
> > include/linux/sframe.h | 2 ++
> > kernel/unwind/sframe.c | 25 +++++++++++++++++++++++--
> > 6 files changed, 50 insertions(+), 3 deletions(-)
>
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > @@ -503,7 +503,7 @@ config HAVE_UNWIND_USER_SFRAME
> >
> > config SFRAME_VALIDATION
> > bool "Enable .sframe section debugging"
> > - depends on HAVE_UNWIND_USER_SFRAME
> > + depends on SFRAME_LOOKUP
>
> depends on UNWIND_SFRAME__LOOKUP
Ah my bad. This mistake was masking similar issues with .init.text and
.exit.text as we had with .rodata.text. I'll send a new version
accounting for those versions as well.
>
> > depends on DYNAMIC_DEBUG
> > help
> > When adding an .sframe section for a task, validate the entire
>
> Regards,
> Jens
> --
> Jens Remus
> Linux on Z Development (D3303)
> jremus@de.ibm.com / jremus@linux.ibm.com
>
> IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
> IBM Data Privacy Statement: https://www.ibm.com/privacy/
>
Thanks,
Dylan
^ permalink raw reply
* Re: [RFC PATCH 2/2] kernel/module: Decouple klp and ftrace from load_module
From: Song Chen @ 2026-04-26 14:26 UTC (permalink / raw)
To: Masami Hiramatsu (Google), Petr Mladek
Cc: Petr Pavlu, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, joe.lawrence, rostedt, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260420112707.aa3627ca9f975eeaf7d8ea0e@kernel.org>
Hi,
On 4/20/26 10:27, Masami Hiramatsu (Google) wrote:
> On Thu, 16 Apr 2026 16:49:32 +0200
> Petr Mladek <pmladek@suse.com> wrote:
>
>> On Thu 2026-04-16 13:18:30, Petr Pavlu wrote:
>>> On 4/15/26 8:43 AM, Song Chen wrote:
>>>> On 4/14/26 22:33, Petr Pavlu wrote:
>>>>> On 4/13/26 10:07 AM, chensong_2000@189.cn wrote:
>>>>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>>>>> index 14f391b186c6..0bdd56f9defd 100644
>>>>>> --- a/include/linux/module.h
>>>>>> +++ b/include/linux/module.h
>>>>>> @@ -308,6 +308,14 @@ enum module_state {
>>>>>> MODULE_STATE_COMING, /* Full formed, running module_init. */
>>>>>> MODULE_STATE_GOING, /* Going away. */
>>>>>> MODULE_STATE_UNFORMED, /* Still setting it up. */
>>>>>> + MODULE_STATE_FORMED,
>>>>>
>>>>> I don't see a reason to add a new module state. Why is it necessary and
>>>>> how does it fit with the existing states?
>>>>>
>>>> because once notifier fails in state MODULE_STATE_UNFORMED (now only ftrace has someting to do in this state), notifier chain will roll back by calling blocking_notifier_call_chain_robust, i'm afraid MODULE_STATE_GOING is going to jeopardise the notifers which don't handle it appropriately, like:
>>>>
>>>> case MODULE_STATE_COMING:
>>>> kmalloc();
>>>> case MODULE_STATE_GOING:
>>>> kfree();
>>>
>>> My understanding is that the current module "state machine" operates as
>>> follows. Transitions marked with an asterisk (*) are announced via the
>>> module notifier.
>>>
>>> ---> UNFORMED --*> COMING --*> LIVE --*> GOING -.
>>> ^ | ^ |
>>> | '---------------------* |
>>> '---------------------------------------'
>>>
>>> The new code aims to replace the current ftrace_module_init() call in
>>> load_module(). To achieve this, it adds a notification for the UNFORMED
>>> state (only when loading a module) and introduces a new FORMED state for
>>> rollback. FORMED is purely a fake state because it never appears in
>>> module::state. The new structure is as follows:
>>>
>>> ,--*> (FORMED)
>>> |
>>> --*> UNFORMED --*> COMING --*> LIVE --*> GOING -.
>>> ^ | ^ |
>>> | '---------------------* |
>>> '---------------------------------------'
>>>
>>> I'm afraid this is quite complex and inconsistent. Unless it can be kept
>>> simple, we would be just replacing one special handling with a different
>>> complexity, which is not worth it.
>>
>>>>>
>>>>>> + if (err)
>>>>>> + goto ddebug_cleanup;
>>>>>> /* Finally it's fully formed, ready to start executing. */
>>>>>> err = complete_formation(mod, info);
>>>>>> - if (err)
>>>>>> + if (err) {
>>>>>> + blocking_notifier_call_chain_reverse(&module_notify_list,
>>>>>> + MODULE_STATE_FORMED, mod);
>>>>>> goto ddebug_cleanup;
>>>>>> + }
>>>>>> - err = prepare_coming_module(mod);
>>>>>> + err = prepare_module_state_transaction(mod,
>>>>>> + MODULE_STATE_COMING, MODULE_STATE_GOING);
>>>>>> if (err)
>>>>>> goto bug_cleanup;
>>>>>> @@ -3522,7 +3519,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>>>>> destroy_params(mod->kp, mod->num_kp);
>>>>>> blocking_notifier_call_chain(&module_notify_list,
>>>>>> MODULE_STATE_GOING, mod);
>>>>>
>>>>> My understanding is that all notifier chains for MODULE_STATE_GOING
>>>>> should be reversed.
>>>> yes, all, from lowest priority notifier to highest.
>>>> I will resend patch 1 which was failed due to my proxy setting.
>>>
>>> What I meant here is that the call:
>>>
>>> blocking_notifier_call_chain(&module_notify_list, MODULE_STATE_GOING, mod);
>>>
>>> should be replaced with:
>>>
>>> blocking_notifier_call_chain_reverse(&module_notify_list, MODULE_STATE_GOING, mod);
>>>
>>>>
>>>>>
>>>>>> - klp_module_going(mod);
>>>>>> bug_cleanup:
>>>>>> mod->state = MODULE_STATE_GOING;
>>>>>> /* module_bug_cleanup needs module_mutex protection */
>>>>>
>>>>> The patch removes the klp_module_going() cleanup call in load_module().
>>>>> Similarly, the ftrace_release_mod() call under the ddebug_cleanup label
>>>>> should be removed and appropriately replaced with a cleanup via
>>>>> a notifier.
>>>>>
>>>> err = prepare_module_state_transaction(mod,
>>>> MODULE_STATE_UNFORMED, MODULE_STATE_FORMED);
>>>> if (err)
>>>> goto ddebug_cleanup;
>>>>
>>>> ftrace will be cleanup in blocking_notifier_call_chain_robust rolling back.
>>>>
>>>> err = prepare_module_state_transaction(mod,
>>>> MODULE_STATE_COMING, MODULE_STATE_GOING);
>>>>
>>>> each notifier including ftrace and klp will be cleanup in blocking_notifier_call_chain_robust rolling back.
>>>>
>>>> if all notifiers are successful in MODULE_STATE_COMING, they all will be clean up in
>>>> coming_cleanup:
>>>> mod->state = MODULE_STATE_GOING;
>>>> destroy_params(mod->kp, mod->num_kp);
>>>> blocking_notifier_call_chain(&module_notify_list,
>>>> MODULE_STATE_GOING, mod);
>>>>
>>>> if something wrong underneath.
>>>
>>> My point is that the patch leaves a call to ftrace_release_mod() in
>>> load_module(), which I expected to be handled via a notifier.
>>
>> I think that I have got it. The ftrace code needs two notifiers when
>> the module is being loaded and two when it is going.
>>
>> This is why Sond added the new state. But I think that we would
>> need two new states to call:
>>
>> + ftrace_module_init() in MODULE_STATE_UNFORMED
>> + ftrace_module_enable() in MODULE_STATE_FORMED
>>
>> and
>>
>> + ftrace_free_mem() in MODULE_STATE_PRE_GOING
>> + ftrace_free_mem() in MODULE_STATE_GOING
>>
>>
>> By using the ascii art:
>>
>> -*> UNFORMED -*> FORMED -> COMING -*> LIVE -*> PRE_GOING -*> GOING -.
>> | | | ^ ^ ^
>> | | '----------------' | |
>> | '--------------------------------------' |
>> '------------------------------------------------------'
>>
>>
>> But I think that this is not worth it.
>
> Agree.
>
> If this needs to be ordered so strictly, why we will use a "single"
> module notifier chain for this complex situation?
>
> I think the notifier call chain is just for notice a single signal,
> instead of sending several different signals, especially if there is
> any dependency among the callbacks.
>
> If notification callbacks need to be ordered, they are currently
> sorted by representing priority numerically, but this is quite
> fragile for updating. It has to look up other registered priorities
> and adjust the order among dependencies each time. For this reason,
> this mechanism is not suitable for global ordering. (It's like line
> numbers in BASIC.)
> It is probably only useful for representing dependencies between
> two components maintained by the same maintainer.
>
> I'm against a general-purpose system that makes everything modular.
> It unnecessarily complicates things. If there are processes that
> require strict ordering, especially processes that must be performed
> before each stage as part of the framework, they should be called
> directly from the framework, not via notification callbacks.
>
> This makes it simpler and more robust to maintain.
>
> Only the framework's end users should utilize notification callbacks.
>
> Thank you,
>
>
my motivation is to decouple ftrace and klp from module loader and make
blocking_notifier_chain more generic, but it doesn't become generic
completely. I understand your and Petr's comments and agree.
Thanks
Best regards
Song
>>
>> Best Regards,
>> Petr
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Song Chen @ 2026-04-26 14:14 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: rafael, lenb, mturquette, sboyd, viresh.kumar, agk, snitzer,
mpatocka, bmarzins, song, yukuai, linan122, jason.wessel, danielt,
dianders, horms, davem, edumazet, kuba, pabeni, paulmck, frederic,
mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, jpoimboe,
jikos, mbenes, pmladek, joe.lawrence, rostedt, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <20260420144429.57b45f2beece690bceea96ec@kernel.org>
Hi Hiramatsu san,
On 4/20/26 13:44, Masami Hiramatsu (Google) wrote:
> Hi Song,
>
> On Wed, 15 Apr 2026 15:01:37 +0800
> chensong_2000@189.cn wrote:
>
>> From: Song Chen <chensong_2000@189.cn>
>>
>> The current notifier chain implementation uses a single-linked list
>> (struct notifier_block *next), which only supports forward traversal
>> in priority order. This makes it difficult to handle cleanup/teardown
>> scenarios that require notifiers to be called in reverse priority order.
>
> What about introducing a new notification callback API that allows you
> to describe dependencies between callback functions?
>
> For example, when registering a callback, you could register a string
> as an ID and specify whether to call it before or after that ID,
> or you could register a comparison function that is called when adding
> to a list. (I prefer @name and @depends fields so that it can be easily
> maintained.)
>
> This would allow for better dependency building when adding to the list.
>
Is the new notification callback API going to replace
blocking_notifier_chain in module loader? or an expansion inside
blocking_notifier_chain but introducing less complexity?
>>
>> A concrete example is the ordering dependency between ftrace and
>> livepatch during module load/unload. see the detail here [1].
>
> If this only concerns notification callback issues with the ftrace
> and livepatch modules, it's far more robust to simply call the
> necessary processing directly when the modules load and unload,
> rather than registering notification callbacks externally.
>
> There are fprobe, kprobe and its trace-events, all of them are using
> ftrace as its fundation layer. In this case, I always needs to
> consider callback order when a module is unloaded.
>
> If ftrace is working as a part of module callbacks, it will conflict
> with fprobe/kprobe module callback. Of course we can reorder it with
> modifying its priority. But this is ugly, because when we introduce
> a new other feature which depends on another layer, we need to
> reorder the callback's priority number on the list.
>
> Based on the above, I don't think this can be resolved simply by
> changing the list of notification callbacks to a bidirectional list.
>
> Thank you,
>
understood, many thanks for your proposal, i will think about it.
best regards,
Song
^ permalink raw reply
* Re: [RFC PATCH 1/2] kernel/notifier: replace single-linked list with double-linked list for reverse traversal
From: Song Chen @ 2026-04-26 13:56 UTC (permalink / raw)
To: Petr Mladek, Masami Hiramatsu
Cc: chensong_2000, rafael, lenb, mturquette, sboyd, viresh.kumar, agk,
snitzer, mpatocka, bmarzins, song, yukuai, linan122, jason.wessel,
danielt, dianders, horms, davem, edumazet, kuba, pabeni, paulmck,
frederic, mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin,
jpoimboe, jikos, mbenes, joe.lawrence, rostedt, mark.rutland,
mathieu.desnoyers, linux-modules, linux-kernel,
linux-trace-kernel, linux-acpi, linux-clk, linux-pm,
live-patching, dm-devel, linux-raid, kgdb-bugreport, netdev
In-Reply-To: <aec90caYZDHDAHgw@pathway.suse.cz>
Hi,
On 4/21/26 17:05, Petr Mladek wrote:
> On Mon 2026-04-20 14:44:29, Masami Hiramatsu wrote:
>> Hi Song,
>>
>> On Wed, 15 Apr 2026 15:01:37 +0800
>> chensong_2000@189.cn wrote:
>>
>>> From: Song Chen <chensong_2000@189.cn>
>>>
>>> The current notifier chain implementation uses a single-linked list
>>> (struct notifier_block *next), which only supports forward traversal
>>> in priority order. This makes it difficult to handle cleanup/teardown
>>> scenarios that require notifiers to be called in reverse priority order.
>>
>> What about introducing a new notification callback API that allows you
>> to describe dependencies between callback functions?
>>
>> For example, when registering a callback, you could register a string
>> as an ID and specify whether to call it before or after that ID,
>> or you could register a comparison function that is called when adding
>> to a list. (I prefer @name and @depends fields so that it can be easily
>> maintained.)
>
> This looks too complex. It would make sense only
> when this API has more users.
>
> Also this won't be enough for the ftrace/livepatch callbacks.
> They need to be ordered against against each other. But they
> also need to be called before/after all other callbacks.
> For example, when the module is loaded:
>
> + 1st frace
> + 2nd livepatch
> + then other notifiers
>
> See the commit c1bf08ac26e92122 ("ftrace: Be first to run code
> modification on modules").
>
>> This would allow for better dependency building when adding to the list.
>
>>>
>>> A concrete example is the ordering dependency between ftrace and
>>> livepatch during module load/unload. see the detail here [1].
>>
>> If this only concerns notification callback issues with the ftrace
>> and livepatch modules, it's far more robust to simply call the
>> necessary processing directly when the modules load and unload,
>> rather than registering notification callbacks externally.
>>
>> There are fprobe, kprobe and its trace-events, all of them are using
>> ftrace as its fundation layer. In this case, I always needs to
>> consider callback order when a module is unloaded.
>>
>> If ftrace is working as a part of module callbacks, it will conflict
>> with fprobe/kprobe module callback. Of course we can reorder it with
>> modifying its priority. But this is ugly, because when we introduce
>> a new other feature which depends on another layer, we need to
>> reorder the callback's priority number on the list.
>>
>> Based on the above, I don't think this can be resolved simply by
>> changing the list of notification callbacks to a bidirectional list.
>
> I agree. I would keep it as is (hardcoded).
>
> Best Regards,
> Petr
>
Thanks for the feedback, the necessity doesn't convincing enough. I will
try the proposal from Masami Hiramatsu.
Best regards,
Song
^ permalink raw reply
* Re: [PATCH] samples/livepatch: Add BPF struct_ops integration sample
From: Yafang Shao @ 2026-04-26 2:35 UTC (permalink / raw)
To: Petr Mladek
Cc: Song Liu, live-patching, linux-kernel, jpoimboe, jikos, mbenes,
joe.lawrence, kernel-team
In-Reply-To: <aedBHr4F0hTsY5x3@pathway.suse.cz>
On Tue, Apr 21, 2026 at 5:19 PM Petr Mladek <pmladek@suse.com> wrote:
>
> On Sun 2026-04-19 11:19:19, Yafang Shao wrote:
> > On Fri, Apr 17, 2026 at 11:52 PM Song Liu <song@kernel.org> wrote:
> > >
> > > On Fri, Apr 17, 2026 at 6:20 AM Petr Mladek <pmladek@suse.com> wrote:
> > > >
> > > > On Thu 2026-04-16 09:32:46, Song Liu wrote:
> > > [...]
> > > > Let' use the code from this patch:
> > > >
> > > > static int __init livepatch_bpf_init(void)
> > > > {
> > > > int ret;
> > > >
> > > > ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS,
> > > > &klp_bpf_kfunc_set);
> > > > ret = ret ?: register_bpf_struct_ops(&bpf_klp_bpf_cmdline_ops,
> > > > klp_bpf_cmdline_ops);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > ---> /*
> > > > ---> * We would need to wait here until the BPF program gets loaded.
> > > > ---> * for the new bpf_struct_ops in this new livepatch.
> > > > ---> */
> >
> > No waiting is necessary. If the BPF program is not attached, the
> > default logic can be executed instead.
>
> But it means a regression. I guess that you need the BPF program
> for a reason. The default logic is not good enough indeed.
>
> > Consider Song's test case: we can handle it as follows.
> >
> > static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
> > {
> > struct klp_bpf_cmdline_ops *ops = READ_ONCE(active_ops);
> >
> > if (ops && ops->set_cmdline)
> > return ops->set_cmdline(m);
> >
> > // If no BPF program is attached, the default kernel function runs.
> > return cmdline_proc_show(m, v);
> > }
> >
> > However, as Song explained below, if we want atomic replace to work,
> > we may need to wait for the new BPF program here. But that would make
> > the combination of livepatch and BPF more complex.
> >
> > Currently, on our production servers, we handle this through a user
> > script, such as:
> >
> > stop_traffic_relying_on_livepatch_bpf
> > kpatch load new-livepatch-bpf-module.ko
> > reattach_the_bpf_program
> > start_the_traffic_again
> >
> > Although this approach requires restarting the affected traffic, other
> > services running on the same server remain unaffected.
>
> We put a lot of effort to make livepatching as less disruptive
> as possible. The atomic replace is supposed to work without
> any disruption.
>
> > > > return klp_enable_patch(&patch);
> > > > }
> > >
> > > Yes, something in this direction is needed to make atomic replace work.
> > > We have no plan to use this in production. I will let Yafang figure out
> > > his plan.
> > >
> > > > Or maybe, the bpf_struct_ops can be _allocated dynamically_ and
> > > > the pointer might be _passed via shadow variables_.
> > > >
> > > > One problem is that shadow variables would add another overhead
> > > > and need not be suitable for hot paths.
> > > >
> > > >
> > > > Anyway, I think that I have similar feelings as Miroslav.
> > > > The combination of livepatches and BPF programs increases
> > > > the complexity for all involved parties: core kernel maintainers,
> > > > livepatch and BPF program authors, and system maintainers.
> > > >
> > > > Do we really want to propagate it?
> > > > Is there any significant advantage in combining these two, please?
> > > > Is it significantly easier to write BPF program then a livepatch?
> > > > Is it significantly easier to update BPF programs then livepatches?
> >
> > This is an important feature for avoiding server restarts,
> > particularly in a VM host environment. Since only one VM on the host
> > may be affected by this feature, we can deploy it rapidly without
> > impacting other VMs on the same host.
>
> This does not answer the question why you need the combination
> of livepatch + BPF. Why a livepatch is not enough?
Consider this recent use case from our production servers:
https://lore.kernel.org/live-patching/CALOAHbDnNba_w_nWH3-S9GAXw0+VKuLTh1gy5hy9Yqgeo4C0iA@mail.gmail.com/
In one of our clusters, we needed to route BGP traffic through
specific NICs based on destination IP addresses. To achieve this
without service interruption, we applied a livepatch to
bond_xmit_3ad_xor_slave_get() to introduce a new hook,
bond_get_slave_hook(). We then attached a BPF program to this hook to
select the outgoing NIC by parsing the SKB. Because the destination
IPs must be adjusted on demand, a static livepatch alone was
insufficient; the BPF integration provided the necessary dynamic
flexibility.
>
> Let me repeat the questions:
>
> Is it significantly easier to write BPF program then a livepatch?
> Is it significantly easier to install BPF programs then livepatches?
>
> > > > Would the support of different replace tags help?
> > > > They would allow to replace only livepatches with the same tag.
> >
> > Right, it will help.
>
> Would this make a rapid update of livepatches easy enough so that
> you won't need the BPF part?
As explained above, we cannot rely solely on livepatching to handle
destination IP changes, as these require real-time updates.
--
Regards
Yafang
^ permalink raw reply
* Re: [PATCH 48/48] objtool/klp: Cache dont_correlate() result
From: Song Liu @ 2026-04-25 1:19 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <c5c4467abea21d4962b4a61c5d6023482e01bd8e.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Cache the dont_correlate() result once per symbol at the start of
> correlate_symbols(). This reduces klp diff time on an arm64 LTO
> vmlinux.o from 2m51s to 35s.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 41/48] objtool/klp: Rewrite symbol correlation algorithm
From: Song Liu @ 2026-04-25 0:53 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <284944d45120ff69959c4d9cde90db13e493d223.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Rewrite the symbol correlation code, using a tiered list of
> deterministic strategies in a loop. For duplicately named symbols, each
> tier applies a filter with the goal of finding a 1:1 deterministic
> correlation between the original and patched version of the symbol.
>
> Overall this works much better than the existing algorithm, particularly
> with LTO kernels.
I found it is hard to follow all the matching algorithms here. Could you
please add some examples for each case: different levels in find_twin(),
match in find_twin_suffixed(), and match in find_twin_positional()?
Also a few nitpicks below.
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
[...]
> +static struct symbol *find_twin(struct elfs *e, struct symbol *sym1)
> +{
> + struct symbol *name_last = NULL, *scope_last = NULL,
> + *file_last = NULL, *csum_last = NULL;
> + unsigned int name_orig = 0, name_patched = 0;
> + unsigned int scope_orig = 0, scope_patched = 0;
> + unsigned int file_orig = 0, file_patched = 0;
> + unsigned int csum_orig = 0, csum_patched = 0;
> + struct symbol *sym2, *match = NULL;
> +
> + /* Count orig candidates */
> + for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) {
> + if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2) ||
> + (!maybe_same_file(sym1, sym2)))
> continue;
>
> - count++;
> - result = sym2;
> + /* Level 1: name match (widest filter) */
> + name_orig++;
> +
> + /* Level 2: scope (scope changes allowed) */
> + if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2))
is_tu_local_sym(sym1) is called many times, shall we add a variable
for it?
> + continue;
> + scope_orig++;
> +
> + /* Level 3: file (scope changes disallowed) */
> + if (!same_file(sym1, sym2))
> + continue;
> + file_orig++;
> +
> + /* Level 4: checksum (unchanged symbols) */
> + if (sym1->len != sym2->len || !sym1->csum.checksum ||
> + sym1->csum.checksum != sym2->csum.checksum)
> + continue;
> + csum_orig++;
> }
>
> - if (count > 1) {
> - ERROR("Multiple (%d) correlation candidates for %s", count, sym->name);
> - return -1;
> + /* Count patched candidates */
> + for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) {
> + if (sym2->twin || sym1->type != sym2->type || dont_correlate(sym2))
> + continue;
> +
> + /* Level 1 */
> + if (!maybe_same_file(sym1, sym2))
> + continue;
This for_each_sym_by_demangled_name() has two "if () continue" while the
first one has one. Maybe keep them the same (just for symmetry)?
Thanks,
Song
> + name_patched++;
> + name_last = sym2;
> +
[...]
^ permalink raw reply
* Re: [PATCH 42/48] objtool/klp: Add correlation debugging output
From: Song Liu @ 2026-04-25 0:30 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <44757e0c259e4651275e24b49dc9f7220ecfe16b.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Add debugging messages to show how duplicate symbols get correlated, and
> split the --debug feature into --debug-correlate and --debug-clone.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 38/48] klp-build: Validate short-circuit prerequisites
From: Song Liu @ 2026-04-25 0:06 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <338295e0bf3353dd62536df672a2615f72be013b.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> The --short-circuit option implicitly requires that certain directories
> are already in klp-tmp. Enforce that to prevent confusing errors.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> scripts/livepatch/klp-build | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index eda690b297cc..b44924d097a5 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build
> @@ -440,6 +440,20 @@ do_init() {
> [[ ! "$SRC" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
> [[ ! "$OBJ" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
>
> + if (( SHORT_CIRCUIT >= 2 )); then
> + [[ -f "$ORIG_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $ORIG_DIR"
> + if (( SHORT_CIRCUIT >= 3 )); then
> + [[ -f "$PATCHED_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $PATCHED_DIR"
> + if (( SHORT_CIRCUIT >= 4 )); then
> + [[ -f "$ORIG_CSUM_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $ORIG_CSUM_DIR"
> + [[ -f "$PATCHED_CSUM_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $PATCHED_CSUM_DIR"
> + if (( SHORT_CIRCUIT >= 5 )); then
> + [[ -f "$DIFF_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $DIFF_DIR"
> + fi
> + fi
> + fi
> + fi
> +
Do we really need these to nest together?
Thanks,
Song
> (( SHORT_CIRCUIT <= 1 )) && rm -rf "$TMP_DIR"
> mkdir -p "$TMP_DIR"
>
> @@ -601,6 +615,7 @@ copy_orig_objects() {
>
> mv -f "$TMP_DIR/build.log" "$ORIG_DIR"
> touch "$TIMESTAMP"
> + touch "$ORIG_DIR/.complete"
> }
[...]
^ permalink raw reply
* Re: [PATCH 39/48] objtool: Replace iterator callbacks with for_each_sym_by_*()
From: Song Liu @ 2026-04-25 0:04 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <03f7804ae62c5c4521053afc3f6a1c4a11bc85de.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Convert the callback-based iterate_sym_by_name() and
> iterate_sym_by_demangled_name() callers to use new
> for_each_sym_by[_demangled]_name() macros. This eliminates the callback
> structs and functions and makes the code more compact and readable.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> tools/objtool/elf.c | 80 ++++++-----------------------
> tools/objtool/include/objtool/elf.h | 40 ++++++++++++---
> tools/objtool/klp-checksum.c | 20 +++-----
> tools/objtool/klp-diff.c | 42 +++++----------
> 4 files changed, 73 insertions(+), 109 deletions(-)
Macros are indeed cleaner. But Sashiko has a valid point on this. [1].
Thanks,
Song
[1] https://sashiko.dev/#/patchset/cover.1776916871.git.jpoimboe%40kernel.org?part=39
^ permalink raw reply
* Re: [PATCH 37/48] objtool/klp: Remove "objtool --checksum"
From: Song Liu @ 2026-04-24 22:25 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <d11a2f7a57690fc4b6a8a02414d07b41dbebeb2b.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> The checksum functionality has been moved to "objtool klp checksum"
> which is now used by klp-build. Remove the now-dead --checksum and
> --debug-checksum options from the default objtool command.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 36/48] klp-build: Use "objtool klp checksum" subcommand
From: Song Liu @ 2026-04-24 22:24 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <99e40357ee24962b3514d9ce4f6e773eff3a15f3.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Use the new "objtool klp checksum" subcommand instead of injecting
> --checksum into every objtool invocation via OBJTOOL_ARGS during the
> kernel build.
>
> This decouples checksum generation from the build, running it in
> separate post-build passes, making the code (and the patch generation
> pipeline itself) more modular.
Having a separate step for checksum removes confusion with
--short-circuit.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 35/48] objtool/klp: Add "objtool klp checksum" subcommand
From: Song Liu @ 2026-04-24 22:18 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <29304d60b4b4949720e3e5a5e6f26196bc29fa07.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Move the checksum functionality out of the main objtool command into a
> new "objtool klp checksum" subcommand.
>
> This has the benefit of making the code (and the patch generation
> process itself) more modular.
>
> For bisectability, both "objtool --checksum" and "objtool klp checksum"
> work for now. The former will be removed after klp-build has been
> converted to use the new subcommand.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 34/48] objtool: Consolidate file decoding into decode_file()
From: Song Liu @ 2026-04-24 22:16 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Josh Poimboeuf, x86, linux-kernel, live-patching, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <20260423084159.GW3126523@noisy.programming.kicks-ass.net>
On Thu, Apr 23, 2026 at 1:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Apr 22, 2026 at 09:04:02PM -0700, Josh Poimboeuf wrote:
> > decode_sections() relies on CFI and cfi_hash initialization done
> > separately in check(), making it unusable outside of check().
> >
> > Consolidate the initialization into decode_sections() and rename it to
> > decode_file(), and make it global along with free_insns() and
> > insn_reloc() for use by other objtool components -- namely, the checksum
> > code which will be moving to another file.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 33/48] objtool/klp: Extricate checksum calculation from validate_branch()
From: Song Liu @ 2026-04-24 22:14 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <ab7a6a65b9139aee9f52829048be928ca0c062b8.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> In preparation for porting the checksum code to other arches, make its
> functionality independent from the CFG reverse engineering code.
>
> Move it into a standalone calculate_checksums() function which iterates
> all functions and instructions directly, rather than being called inline
> from do_validate_branch().
>
> Since checksum_update_insn() is no longer called during CFG traversal,
> it needs to manually iterate the alternatives.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 31/48] objtool: Add is_alias_sym() helper
From: Song Liu @ 2026-04-24 22:10 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Josh Poimboeuf, x86, linux-kernel, live-patching, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <20260423083500.GU3126523@noisy.programming.kicks-ass.net>
On Thu, Apr 23, 2026 at 1:35 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Apr 22, 2026 at 09:03:59PM -0700, Josh Poimboeuf wrote:
> > Improve readability with a new is_alias_sym() helper.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 30/48] objtool/klp: Handle Clang .data..Lanon anonymous data sections
From: Song Liu @ 2026-04-24 22:09 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <11a0af398f5ebd591e87f3f8627bbf512260549a.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> Clang generates anonymous data sections named .data..Lanon.<hash>.
> These need section-symbol references in the same way as .data..Lubsan
> (GCC) and .data..L__unnamed_ (Clang UBSAN) sections. Without this,
> convert_reloc_sym() fails when processing relocations that reference
> these sections.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ permalink raw reply
* Re: [PATCH 28/48] objtool/klp: Create empty checksum sections for function-less object files
From: Song Liu @ 2026-04-24 22:08 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <199a3d975e8e562421edd342b9eda242b4f57a71.1776916871.git.jpoimboe@kernel.org>
On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> If an object file has no functions, objtool has nothing to checksum, so
> it doesn't create the .discard.sym_checksum symbol.
>
> Then when 'objtool klp diff' reads symbol checksums, it errors out due
> to the missing .discard.sym_checksum section.
>
> Instead, just create an empty checksum section to signal to
> read_sym_checksums() that the file has been processed.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Acked-by: Song Liu <song@kernel.org>
^ 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