* [PATCH v5 0/8] unwind, arm64: add sframe unwinder for kernel
From: Dylan Hatch @ 2026-04-28 18:36 UTC (permalink / raw)
To: Roman Gushchin, Weinan Liu, Will Deacon, Josh Poimboeuf,
Indu Bhagat, Peter Zijlstra, Steven Rostedt, Catalin Marinas,
Jiri Kosina, Jens Remus
Cc: Dylan Hatch, Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan,
Song Liu, joe.lawrence, linux-toolchains, linux-kernel,
live-patching, linux-arm-kernel, Randy Dunlap
Implement a generic kernel sframe-based [1] unwinder. The main goal is
to improve reliable stacktrace on arm64 by unwinding across exception
boundaries.
On x86, the ORC unwinder provides reliable stacktrace through similar
methodology, but arm64 lacks the necessary support from objtool to
create ORC unwind tables.
Currently, there's already a sframe unwinder proposed for userspace: [2].
To maintain common definitions and algorithms for sframe lookup, a
substantial portion of this patch series aims to refactor the sframe
lookup code to support both kernel and userspace sframe sections.
Currently, only GNU Binutils support sframe. This series relies on the
Sframe V3 format, which is supported in binutils 2.46.
These patches are based on Steven Rostedt's sframe/core branch [3],
which is and aggregation of existing work done for x86 sframe userspace
unwind, and contains [2]. This branch is, in turn, based on Linux
v7.0-rc3. This full series (applied to the sframe/core branch) is
available on github: [4].
Ref:
[1]: https://sourceware.org/binutils/docs/sframe-spec.html
[2]: https://lore.kernel.org/lkml/20260127150554.2760964-1-jremus@linux.ibm.com/
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git/log/?h=sframe/core
[4]: https://github.com/dylanbhatch/linux/tree/sframe-v5
Changes since v4:
- (Jens) Fix some minor nits.
- Handle .init.text and .exit.text in function address validation.
Changes since v3:
- (Jens) Clean up patch summaries.
- (Jens) Rename SFRAME_LOOKUP -> UNWIND_SFRAME_LOOKUP to fit existing
naming convention.
- (Randy) Correct typo errors in new config options.
- (Jens) Move unwind types to a new unwind_types.h to match their
usage.
- (Jens) Update KERNEL_[COPY|GET] to use label-based error handling
like their userspace counterparts.
- (Jens) Rename SFRAME_UNWINDER -> HAVE_UNWIND_KERNEL_SFRAME and
ARCH_SUPPORTS_SFRAME_UNWINDER -> ARCH_SUPPORTS_UNWIND_KERNEL_SFRAME
to match existing naming convention.
- (Jens) Move HAVE_UNWIND_KERNEL_SFRAME config option to arch/Kconfig.
- (Jens) Rename/move extern definitions of __[start|end]_sframe into
include/asm-generic/sections.h.
- (Jens) Fix up CFI annotations at kernel entry.
- (Jens) Fix error path for unsorted FDE lookup.
- (Jens) Zero-out module sframe_section before init.
- (Jens) For SFRAME_VALIDATION, use an arch-specific function-address
validation helper so that .rodata.text can be correctly handled on
arm64 vmlinux.
- (Jens) Fixup and better comment kernel stacktrace code.
Changes since v2:
The biggest change from v2 is the switch from adding a dedicated,
in-kernel sframe-lookup library, to refactoring/using the existing
library developed by Josh, Jens, and Steve. Consequently, this series
now depends on Sframe V3, though this upgrade would likely have been
necessary anyway. Below is a full accounting of the changes since v2.
- (Josh) Add stricter reliability checks during unwind.
- (Puranjay, Indu, Jens) Update to use a common sframe library with
userpace unwind, thus resolving the need to support
SFRAME_F_FDE_FUNC_START_PCREL, added in binutils 2.45.
- (Jens) Add check for sframe V3, thus resolving the prior need for V2
and SFRAME_F_FDE_FUNC_START_PCREL support.
- (Will) Add ARCH_SUPPORTS_SFRAME_UNWINDER, remove SFRAME_UNWIND_TABLE
- (Indu) add support for unsorted FDE tables, allowing for module
sframe lookups.
- (Mark) Prefer frame-pointer unwind when possible, for better
performance.
- Simplify compile-time logic, adding stubbs when necessary.
- Add support for in-kernel SFRAME_VALIDATION.
- Rebase onto core/sframe (with v7.0-rc3 base)
Dylan Hatch (7):
sframe: Allow kernelspace sframe sections
arm64, unwind: build kernel with sframe V3 info
sframe: Provide PC lookup for vmlinux .sframe section
sframe: Allow unsorted FDEs
arm64/module, sframe: Add sframe support for modules
sframe: Introduce in-kernel SFRAME_VALIDATION
unwind: arm64: Use sframe to unwind interrupt frames
Weinan Liu (1):
arm64: entry: add unwind info for various kernel entries
MAINTAINERS | 3 +-
Makefile | 8 +
arch/Kconfig | 27 +-
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/module.h | 6 +
arch/arm64/include/asm/sections.h | 1 +
arch/arm64/include/asm/stacktrace/common.h | 6 +
arch/arm64/include/asm/unwind_sframe.h | 55 +++
arch/arm64/kernel/entry.S | 23 +
arch/arm64/kernel/module.c | 8 +
arch/arm64/kernel/setup.c | 2 +
arch/arm64/kernel/stacktrace.c | 246 ++++++++++-
arch/arm64/kernel/vdso/Makefile | 2 +-
arch/arm64/kernel/vmlinux.lds.S | 2 +
.../{unwind_user_sframe.h => unwind_sframe.h} | 6 +-
arch/x86/include/asm/unwind_user.h | 12 +-
include/asm-generic/sections.h | 4 +
include/asm-generic/vmlinux.lds.h | 15 +
include/linux/sframe.h | 67 ++-
include/linux/unwind_types.h | 46 ++
include/linux/unwind_user_types.h | 41 --
kernel/unwind/Makefile | 2 +-
kernel/unwind/sframe.c | 410 ++++++++++++++----
kernel/unwind/user.c | 41 +-
24 files changed, 827 insertions(+), 207 deletions(-)
create mode 100644 arch/arm64/include/asm/unwind_sframe.h
rename arch/x86/include/asm/{unwind_user_sframe.h => unwind_sframe.h} (50%)
create mode 100644 include/linux/unwind_types.h
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply
* Re: [PATCH 10/48] objtool/klp: Fix --debug-checksum for duplicate symbol names
From: Josh Poimboeuf @ 2026-04-28 16:30 UTC (permalink / raw)
To: Miroslav Benes
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Petr Mladek
In-Reply-To: <177737835356.11371.5834023067101378921.b4-review@b4>
On Tue, Apr 28, 2026 at 02:12:33PM +0200, Miroslav Benes wrote:
> On Wed, 22 Apr 2026 21:03:38 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > find_symbol_by_name() only returns the first match, so
> > --debug-checksum=<func> silently ignores any subsequent duplicately
> > named functions after the first.
> >
> > Add a new iterate_sym_by_name() to fix that.
>
> The patch is ok but is it worth it when you remove it again in 39/48
> with a better implementation? Can 39/48 be moved here in the series
> instead?
Indeed...
--
Josh
^ permalink raw reply
* Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT
From: Josh Poimboeuf @ 2026-04-28 16:25 UTC (permalink / raw)
To: David Laight
Cc: Peter Zijlstra, x86, linux-kernel, live-patching, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <20260428023717.6a7c68c6@pumpkin>
On Tue, Apr 28, 2026 at 02:37:17AM +0100, David Laight wrote:
> > static const struct option check_options[] = {
> > OPT_GROUP("Actions:"),
> > - OPT_BOOLEAN(0, "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
> > OPT_STRING_OPTARG('d', "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
> > OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
> > OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
> > @@ -84,7 +83,7 @@ static const struct option check_options[] = {
> > OPT_BOOLEAN('r', "retpoline", &opts.retpoline, "validate and annotate retpoline usage"),
> > OPT_BOOLEAN(0, "rethunk", &opts.rethunk, "validate and annotate rethunk usage"),
> > OPT_BOOLEAN(0, "unret", &opts.unret, "validate entry unret placement"),
> > - OPT_INTEGER(0, "prefix", &opts.prefix, "generate prefix symbols"),
> > + OPT_INTEGER(0, "prefix", &opts.prefix, "generate or grow prefix symbols for N-byte function padding"),
> > OPT_BOOLEAN('l', "sls", &opts.sls, "validate straight-line-speculation mitigations"),
> > OPT_BOOLEAN('s', "stackval", &opts.stackval, "validate frame pointer rules"),
> > OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"),
> > @@ -92,6 +91,7 @@ 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, "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 +163,11 @@ static bool opts_valid(void)
> > return false;
> > }
> >
> > + if (opts.cfi && !opts.prefix) {
> > + ERROR("--cfi requires --prefix");
> > + return false;
> > + }
> > +
>
> Wouldn't it be more friendly to have:
> opts.prefix |= opts.cfi;
> and change the help to (implies --prefix).
It would, if prefix didn't take an integer as an argument ;-)
--
Josh
^ permalink raw reply
* Re: [PATCH 41/48] objtool/klp: Rewrite symbol correlation algorithm
From: Josh Poimboeuf @ 2026-04-28 16:23 UTC (permalink / raw)
To: Song Liu
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <CAPhsuW4zF9P1mpZ88P5k=TgbHSCQfrdqFok9YiANHo5CLttZNQ@mail.gmail.com>
On Fri, Apr 24, 2026 at 05:53:47PM -0700, Song Liu wrote:
> 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()?
Ack.
> 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?
Unless it's actually a performance issue I'd prefer not to add yet
another bit to struct symbol.
>
> > + 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)?
Oops, asymmetry not intended there, thanks.
--
Josh
^ permalink raw reply
* Re: [PATCH 38/48] klp-build: Validate short-circuit prerequisites
From: Josh Poimboeuf @ 2026-04-28 16:19 UTC (permalink / raw)
To: Song Liu
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <CAPhsuW66Mx6RU9NT4nyk7GuN-0Kve6P7YdnM-ke3D2Cx0o0_Kw@mail.gmail.com>
On Fri, Apr 24, 2026 at 05:06:43PM -0700, Song Liu wrote:
> 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?
Hm, I suppose flattening is a little less offensive to the eye:
if (( SHORT_CIRCUIT >= 2 )); then
[[ -f "$ORIG_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $ORIG_DIR"
fi
if (( SHORT_CIRCUIT >= 3 )); then
[[ -f "$PATCHED_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $PATCHED_DIR"
fi
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"
fi
if (( SHORT_CIRCUIT >= 5 )); then
[[ -f "$DIFF_DIR/.complete" ]] || die "-S $SHORT_CIRCUIT requires completed $DIFF_DIR"
fi
--
Josh
^ permalink raw reply
* Re: [PATCH 39/48] objtool: Replace iterator callbacks with for_each_sym_by_*()
From: Josh Poimboeuf @ 2026-04-28 16:14 UTC (permalink / raw)
To: Song Liu
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <CAPhsuW7=EAeOT3cTvROoDVcvU6wibF56nJFPQzXQhnVcxfmPrw@mail.gmail.com>
On Fri, Apr 24, 2026 at 05:04:08PM -0700, Song Liu wrote:
> 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].
Ok, I'll make those an "if (mismatch) {} else":
#define for_each_sym_by_name(elf, _name, sym) \
elf_hash_for_each_possible(elf, symbol_name, sym, name_hash, \
str_hash_demangled(_name)) \
if (strcmp(sym->name, _name)) {} else
--
Josh
^ permalink raw reply
* Re: [PATCH 18/48] klp-build: Fix hang on out-of-date .config
From: Josh Poimboeuf @ 2026-04-28 15:57 UTC (permalink / raw)
To: Song Liu
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <CAPhsuW7c44JzDX8Q6PgYAeigSFWTuEnuvgG88ABDdMNmPY6Oiw@mail.gmail.com>
On Fri, Apr 24, 2026 at 02:51:54PM -0700, Song Liu wrote:
> On Wed, Apr 22, 2026 at 9:04 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > If .config is out of date with the kernel source, 'make syncconfig'
> > hangs while waiting for user input on new config options. Detect the
> > mismatch and return an error.
> >
> > Fixes: 6f93f7b06810 ("livepatch/klp-build: Fix inconsistent kernel version")
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > ---
> > scripts/livepatch/klp-build | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> > index 0ad7e6631314..81b35fc10877 100755
> > --- a/scripts/livepatch/klp-build
> > +++ b/scripts/livepatch/klp-build
> > @@ -306,7 +306,12 @@ set_kernelversion() {
> >
> > stash_file "$file"
> >
> > - kernelrelease="$(cd "$SRC" && make syncconfig &>/dev/null && make -s kernelrelease)"
> > + if [[ -n "$(make -s listnewconfig 2>/dev/null)" ]]; then
> > + die ".config mismatch, check your .config or run 'make olddefconfig'"
> > + fi
> > + make syncconfig &>/dev/null || die "make syncconfig failed"
> > +
> > + kernelrelease="$(cd "$SRC" && make -s kernelrelease)"
>
> Do we really need cd "$SRC" here? If so, we need it before
> all the make commands, right?
Yeah, the "$SRC" thing is a half-hearted implementation throughout, with
the idea of eventually having the sourcedir outside of $PWD, and/or
building the object files in a separate directory. That's confusing,
I'll just strip all that out for now.
--
Josh
^ permalink raw reply
* Re: [PATCH 11/48] objtool/klp: Fix handling of zero-length .altinstr_replacement sections
From: Josh Poimboeuf @ 2026-04-28 15:49 UTC (permalink / raw)
To: Song Liu
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Miroslav Benes, Petr Mladek
In-Reply-To: <CAPhsuW53h4mCj8=0bfGy5N8O9JCUMGgCZTpMY8c140wno+UPXQ@mail.gmail.com>
On Fri, Apr 24, 2026 at 02:19:52PM -0700, Song Liu wrote:
> On Wed, Apr 22, 2026 at 9:05 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > When a section is empty (e.g. only zero-length alternative
> > replacements), there are no symbols to convert a section symbol
> > reference to. Skip the reloc instead of erroring out.
> >
> > Fixes: dd590d4d57eb ("objtool/klp: Introduce klp diff subcommand for diffing object files")
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
>
> Acked-by: Song Liu <song@kernel.org>
>
> With a nitpick below:
>
> [...]
>
> > @@ -1293,12 +1301,15 @@ static int clone_sym_relocs(struct elfs *e, struct symbol *patched_sym)
> > !strcmp(patched_reloc->sym->sec->name, ".altinstr_aux"))
> > continue;
> >
> > - if (convert_reloc_sym(e->patched, patched_reloc)) {
> > + ret = convert_reloc_sym(e->patched, patched_reloc);
> > + if (ret < 0) {
> > ERROR_FUNC(patched_rsec->base, reloc_offset(patched_reloc),
> > "failed to convert reloc sym '%s' to its proper format",
> > patched_reloc->sym->name);
> > return -1;
> > }
> > + if (ret > 0)
> > + continue;
>
> Functions that return -1, 0, 1 are usually more confusing. Shall we add more
> comments for convert_reloc_sym()?
Indeed, thanks.
--
Josh
^ permalink raw reply
* Re: [PATCH 06/48] objtool/klp: Don't correlate rodata symbols
From: Josh Poimboeuf @ 2026-04-28 15:46 UTC (permalink / raw)
To: Miroslav Benes
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Petr Mladek
In-Reply-To: <177703167198.234971.16690298062704654470.b4-review@b4>
On Fri, Apr 24, 2026 at 01:54:31PM +0200, Miroslav Benes wrote:
> On Wed, 22 Apr 2026 21:03:34 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> > index ea9ccf8c4ea9..f6597015b33b 100644
> > --- a/tools/objtool/klp-diff.c
> > +++ b/tools/objtool/klp-diff.c
> > @@ -374,6 +374,7 @@ static bool dont_correlate(struct symbol *sym)
> > is_uncorrelated_static_local(sym) ||
> > is_clang_tmp_label(sym) ||
> > is_string_sec(sym->sec) ||
> > + is_rodata_sec(sym->sec) ||
>
> Sashiko comments here [1] that the check is suddenly to broad and it
> covers also global rodata symbols which might then be skipped in
> klp_reloc_needed(). I think it has a point.
>
> [1] https://sashiko.dev/#/patchset/cover.1776916871.git.jpoimboe%40kernel.org?part=6
Yeah, after looking at this, I'm going to drop this patch. While
there's no harm in duplicating read-only data, it would break pointer
comparison, regardless of global or local.
--
Josh
^ permalink raw reply
* Re: [PATCH 17/48] objtool: Fix reloc hash collision in find_reloc_by_dest_range()
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <09dab1995c4ba6ca29dd70b0a7472f1a2975fefd.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:45 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> In find_reloc_by_dest_range(), hash collisions can cause a high-offset
> relocation to appear when probing a low-offset hash bucket.
>
> Only return early when the best match found so far genuinely belongs to
> the current bucket (its offset is within the bucket's stride range).
> Otherwise, continue scanning later buckets which may contain
> lower-offset matches.
>
> [...]
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 16/48] objtool/klp: Fix relocation conversion failures for R_X86_64_NONE
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <2779114efd74a9dd9f1e78076e1b9e3a5273de73.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:44 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> Objtool has some hacks which NOP out certain calls/jumps and replace
> their relocations with R_X86_64_NONE. The klp-diff relocation
> extraction code will error out when trying to copy these relocations due
> to their negative addend, which would only makes sense for a PC-relative
> branch instruction. Just ignore them.
>
>
> [...]
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 15/48] objtool/klp: Fix kCFI trap handling
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <2022e064d670290bfc4ce96207c64b2282d39959.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:43 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> .kcfi_traps contains references to kCFI trap instruction locations.
> When a KCFI type check fails at an indirect call, the trap handler looks
> up the faulting address in this section.
>
> Add it to the special sections list so the entries get extracted for the
> changed functions they reference.
>
> [...]
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 14/48] objtool/klp: Fix extraction of text annotations for alternatives
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <5e67de043745aec66abf963edbd74d13c5ea142a.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:42 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> Objtool is failing to extract text annotations which reference
> .altinstr_replacement instructions:
>
> 1) Alternative replacement fake symbols are NOTYPE rather than FUNC,
> and they don't have sym->included set, thus they aren't recognized
> by should_keep_special_sym().
>
> [...]
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 13/48] objtool/klp: Fix XXH3 state memory leak
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <b998db762616ed3c4972b64a3f64759d39bfe674.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:41 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> The XXH3 state allocated in checksum_init() is never freed. Free it in
> checksum_finish().
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 12/48] objtool/klp: Fix cloning of zero-length section symbols
From: Miroslav Benes @ 2026-04-28 12:42 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <2a02cb0d5de7a60f5ef135dac071c93f6303bd82.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:40 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> Fix NULL dereference when cloning a symbol from an empty section.
> sec->data is only populated for sections with non-zero size.
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 11/48] objtool/klp: Fix handling of zero-length .altinstr_replacement sections
From: Miroslav Benes @ 2026-04-28 12:12 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <99099e77dffb352f97c5276298ab344c186a3ee2.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:39 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> When a section is empty (e.g. only zero-length alternative
> replacements), there are no symbols to convert a section symbol
> reference to. Skip the reloc instead of erroring out.
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH 10/48] objtool/klp: Fix --debug-checksum for duplicate symbol names
From: Miroslav Benes @ 2026-04-28 12:12 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <7fd49264db4f5a9c654ad162cca96ce575e77ae4.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:38 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> find_symbol_by_name() only returns the first match, so
> --debug-checksum=<func> silently ignores any subsequent duplicately
> named functions after the first.
>
> Add a new iterate_sym_by_name() to fix that.
The patch is ok but is it worth it when you remove it again in 39/48
with a better implementation? Can 39/48 be moved here in the series
instead?
--
Miroslav
^ permalink raw reply
* Re: [PATCH 09/48] objtool/klp: Fix create_fake_symbols() skipping entsize-based sections
From: Miroslav Benes @ 2026-04-28 11:45 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <e10231fce5d8f3f17e4cc7a396a4a8e8d791f994.1776916871.git.jpoimboe@kernel.org>
On Wed, 22 Apr 2026 21:03:37 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> create_fake_symbols() has two phases: creating symbols from
> ANNOTATE_DATA_SPECIAL entries, and a fallback that uses sh_entsize for
> special sections like .static_call_sites.
>
> When .discard.annotate_data is absent, the function returns early,
> skipping the entsize fallback and silently allowing unsupported
> module-local static call keys through.
>
> [...]
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
--
Miroslav
^ permalink raw reply
* Re: [PATCH v2 2/2] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-04-28 8:23 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: <88ae41dc-e5e0-442a-9b95-5125adf31e75@suse.com>
On Mon, Apr 27, 2026 at 03:51:31PM +0200, Petr Pavlu wrote:
> 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'.
Good point.
> >>> /* 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.
I personally prefer not to over-engineer for cases that may never occur.
That said, your concerns about edge cases, consistency, and livepatch
handling are valid.
Let me take some time to think this through and experiment with possible
solutions that handle these cases properly. I can’t work on this
immediately, but I’ll get back to it.
Regards
Stanislaw
^ permalink raw reply
* Re: [PATCH 03/48] objtool/klp: Don't correlate __ADDRESSABLE() symbols
From: Miroslav Benes @ 2026-04-28 6:35 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Petr Mladek
In-Reply-To: <3ca4kejjaoy4kj4p2232xtb6rpgog5q7dm65ljq5tvqwp6liij@yfxb3womgfnv>
On Mon, 27 Apr 2026, Josh Poimboeuf wrote:
> On Fri, Apr 24, 2026 at 11:34:33AM +0200, Miroslav Benes wrote:
> > On Fri, 24 Apr 2026, Miroslav Benes wrote:
> >
> > > On Wed, 22 Apr 2026 21:03:31 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > Symbols created by __ADDRESSABLE() are only used to convince the
> > > > toolchain not to optimize out the referenced symbol.
> > >
> > > Reviewed-by: Miroslav Benes <mbenes@suse.cz>
> >
> > Looking at it again... wouldn't it be better to address this in
> > is_special_section() which is looking at .discard.addressable already
> > (only the outcome is different)?
>
> No, I don't think so. If .discard.addressable were classified as a
> "special" section then klp-diff would try to extract its entries into
> the livepatch module, which is completely unnecessary as these are
> throwaway symbols.
True. Thanks.
Miroslav
^ permalink raw reply
* Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT
From: David Laight @ 2026-04-28 1:37 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Peter Zijlstra, x86, linux-kernel, live-patching, Joe Lawrence,
Song Liu, Miroslav Benes, Petr Mladek
In-Reply-To: <c7vi7gpfrybjmngjoqu2jmirh6jp53bpw5edeoeupz5gwhw6gx@fvcn6l6vgl47>
On Thu, 23 Apr 2026 20:38:02 -0700
Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> On Thu, Apr 23, 2026 at 04:30:47PM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 09:23:12AM -0700, Josh Poimboeuf wrote:
> > > On Thu, Apr 23, 2026 at 05:19:25PM +0200, Peter Zijlstra wrote:
> > > > On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:
> > > > > On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:
> > > > > > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > > > > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > > > > > __pfx_ symbols were considered redundant.
> > > > > > >
> > > > > > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash. With
> > > > > > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > > > > > the hash and the function entry which have no symbol to claim them.
> > > > > >
> > > > > > If you force the function alignment to 64 bytes, the prefix will also be
> > > > > > 64bytes, rather than the normal 16.
> > > > >
> > > > > Sorry, how do you get 64 here?
> > > >
> > > > DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y
> > >
> > > Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
> > > Or a 64-byte pfx for the !CFI case.
> > >
> > > > > > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > > > > > Without a symbol, unwinders and other tools that symbolize code
> > > > > > > locations misattribute those bytes.
> > > > > > >
> > > > > > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > > > > > CALL_PADDING configs, covering the full padding area regardless of
> > > > > > > whether there's also a __cfi_ symbol.
> > > > > >
> > > > > > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > > > > > __cfi_ symbols to cover the whole prefix?
> > > > >
> > > > > Yeah, I suppose that would be better, via objtool I presume.
> > > >
> > > > Yup.
>
> I discovered it's not just FineIBT, it's basically any CALL_PADDING+CFI,
> like so:
>
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Subject: [PATCH] objtool: Grow __cfi_* symbols for all kCFI+CALL_PADDING
>
> For all CONFIG_CFI+CONFIG_CALL_PADDING configs, the __cfi_ symbols only
> cover the 5-byte kCFI type hash. After that there also N bytes of NOP
> padding between the hash and the function entry which aren't associated
> with any symbol.
>
> The NOPs can be replaced with actual code at runtime. Without a symbol,
> unwinders and tooling have no way of knowing where those bytes belong.
>
> Grow the existing __cfi_* symbols to fill that gap.
>
> Also, CONFIG_PREFIX_SYMBOLS has no reason to exist: CONFIG_CALL_PADDING
> is what causes the compiler to emit NOP padding before function entry
> (via -fpatchable-function-entry), so it's the right condition for
> creating prefix symbols.
>
> Remove CONFIG_PREFIX_SYMBOLS, as it's no longer needed. Simplify the
> LONGEST_SYM_KUNIT_TEST dependency accordingly.
>
> Update the --cfi and --prefix usage strings to reflect their current
> scope.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
...
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index ec7f10a5ef19..254ceb6b0e2c 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -73,7 +73,6 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
>
> static const struct option check_options[] = {
> OPT_GROUP("Actions:"),
> - OPT_BOOLEAN(0, "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
> OPT_STRING_OPTARG('d', "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
> OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
> OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
> @@ -84,7 +83,7 @@ static const struct option check_options[] = {
> OPT_BOOLEAN('r', "retpoline", &opts.retpoline, "validate and annotate retpoline usage"),
> OPT_BOOLEAN(0, "rethunk", &opts.rethunk, "validate and annotate rethunk usage"),
> OPT_BOOLEAN(0, "unret", &opts.unret, "validate entry unret placement"),
> - OPT_INTEGER(0, "prefix", &opts.prefix, "generate prefix symbols"),
> + OPT_INTEGER(0, "prefix", &opts.prefix, "generate or grow prefix symbols for N-byte function padding"),
> OPT_BOOLEAN('l', "sls", &opts.sls, "validate straight-line-speculation mitigations"),
> OPT_BOOLEAN('s', "stackval", &opts.stackval, "validate frame pointer rules"),
> OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"),
> @@ -92,6 +91,7 @@ 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, "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 +163,11 @@ static bool opts_valid(void)
> return false;
> }
>
> + if (opts.cfi && !opts.prefix) {
> + ERROR("--cfi requires --prefix");
> + return false;
> + }
> +
Wouldn't it be more friendly to have:
opts.prefix |= opts.cfi;
and change the help to (implies --prefix).
David
^ permalink raw reply
* Re: [PATCH 03/48] objtool/klp: Don't correlate __ADDRESSABLE() symbols
From: Josh Poimboeuf @ 2026-04-27 19:00 UTC (permalink / raw)
To: Miroslav Benes
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Petr Mladek
In-Reply-To: <alpine.LSU.2.21.2604241133330.25696@pobox.suse.cz>
On Fri, Apr 24, 2026 at 11:34:33AM +0200, Miroslav Benes wrote:
> On Fri, 24 Apr 2026, Miroslav Benes wrote:
>
> > On Wed, 22 Apr 2026 21:03:31 -0700, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > Symbols created by __ADDRESSABLE() are only used to convince the
> > > toolchain not to optimize out the referenced symbol.
> >
> > Reviewed-by: Miroslav Benes <mbenes@suse.cz>
>
> Looking at it again... wouldn't it be better to address this in
> is_special_section() which is looking at .discard.addressable already
> (only the outcome is different)?
No, I don't think so. If .discard.addressable were classified as a
"special" section then klp-diff would try to extract its entries into
the livepatch module, which is completely unnecessary as these are
throwaway symbols.
--
Josh
^ permalink raw reply
* [PATCH v3 6/6] selftests: livepatch: Check if stack_order 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 3dae09de4061 ("livepatch: Add stack_order sysfs attribute"),
merged in v6.14, introduced 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 | 43 ++++++++++++++-----------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
index 744c612a90d3..ce67a2b770d0 100755
--- a/tools/testing/selftests/livepatch/test-sysfs.sh
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -10,6 +10,7 @@ MOD_LIVEPATCH3=test_klp_syscall
HAS_PATCH_ATTR=0
HAS_REPLACE_ATTR=0
+HAS_STACK_ORDER_ATTR=0
setup_config
@@ -23,8 +24,6 @@ check_sysfs_rights "$MOD_LIVEPATCH" "" "drwxr-xr-x"
check_sysfs_rights "$MOD_LIVEPATCH" "enabled" "-rw-r--r--"
check_sysfs_value "$MOD_LIVEPATCH" "enabled" "1"
check_sysfs_rights "$MOD_LIVEPATCH" "force" "--w-------"
-check_sysfs_rights "$MOD_LIVEPATCH" "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"
@@ -39,6 +38,12 @@ if does_sysfs_exist "$MOD_LIVEPATCH" "replace"; then
HAS_REPLACE_ATTR=1
fi
+if does_sysfs_exist "$MOD_LIVEPATCH" "stack_order"; then
+ check_sysfs_rights "$MOD_LIVEPATCH" "stack_order" "-r--r--r--"
+ check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
+ HAS_STACK_ORDER_ATTR=1
+fi
+
disable_lp $MOD_LIVEPATCH
unload_lp $MOD_LIVEPATCH
@@ -150,33 +155,34 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
% rmmod $MOD_LIVEPATCH"
fi
-start_test "sysfs test stack_order value"
+if [[ "$HAS_STACK_ORDER_ATTR" == "1" ]]; then
+ start_test "sysfs test stack_order value"
-load_lp $MOD_LIVEPATCH
+ load_lp $MOD_LIVEPATCH
-check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
+ check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
-load_lp $MOD_LIVEPATCH2
+ load_lp $MOD_LIVEPATCH2
-check_sysfs_value "$MOD_LIVEPATCH2" "stack_order" "2"
+ check_sysfs_value "$MOD_LIVEPATCH2" "stack_order" "2"
-load_lp $MOD_LIVEPATCH3
+ load_lp $MOD_LIVEPATCH3
-check_sysfs_value "$MOD_LIVEPATCH3" "stack_order" "3"
+ check_sysfs_value "$MOD_LIVEPATCH3" "stack_order" "3"
-disable_lp $MOD_LIVEPATCH2
-unload_lp $MOD_LIVEPATCH2
+ disable_lp $MOD_LIVEPATCH2
+ unload_lp $MOD_LIVEPATCH2
-check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
-check_sysfs_value "$MOD_LIVEPATCH3" "stack_order" "2"
+ check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
+ check_sysfs_value "$MOD_LIVEPATCH3" "stack_order" "2"
-disable_lp $MOD_LIVEPATCH3
-unload_lp $MOD_LIVEPATCH3
+ disable_lp $MOD_LIVEPATCH3
+ unload_lp $MOD_LIVEPATCH3
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
+ disable_lp $MOD_LIVEPATCH
+ unload_lp $MOD_LIVEPATCH
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
+ check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
livepatch: enabling patch '$MOD_LIVEPATCH'
livepatch: '$MOD_LIVEPATCH': initializing patching transition
livepatch: '$MOD_LIVEPATCH': starting patching transition
@@ -216,5 +222,6 @@ livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
livepatch: '$MOD_LIVEPATCH': unpatching complete
% rmmod $MOD_LIVEPATCH"
+fi
exit 0
--
2.54.0
^ permalink raw reply related
* [PATCH v3 5/6] selftests: livepatch: Check if replace 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 adb68ed26a3e ("livepatch: Add "replace" sysfs attribute"),
merged in v6.11, introduced 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 | 39 +++++++++++++++----------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
index 394cf3ff99cd..744c612a90d3 100755
--- a/tools/testing/selftests/livepatch/test-sysfs.sh
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -9,6 +9,7 @@ MOD_LIVEPATCH2=test_klp_callbacks_demo
MOD_LIVEPATCH3=test_klp_syscall
HAS_PATCH_ATTR=0
+HAS_REPLACE_ATTR=0
setup_config
@@ -22,7 +23,6 @@ check_sysfs_rights "$MOD_LIVEPATCH" "" "drwxr-xr-x"
check_sysfs_rights "$MOD_LIVEPATCH" "enabled" "-rw-r--r--"
check_sysfs_value "$MOD_LIVEPATCH" "enabled" "1"
check_sysfs_rights "$MOD_LIVEPATCH" "force" "--w-------"
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
check_sysfs_rights "$MOD_LIVEPATCH" "stack_order" "-r--r--r--"
check_sysfs_value "$MOD_LIVEPATCH" "stack_order" "1"
check_sysfs_rights "$MOD_LIVEPATCH" "transition" "-r--r--r--"
@@ -34,6 +34,11 @@ if does_sysfs_exist "$MOD_LIVEPATCH/vmlinux" "patched"; then
HAS_PATCH_ATTR=1
fi
+if does_sysfs_exist "$MOD_LIVEPATCH" "replace"; then
+ check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
+ HAS_REPLACE_ATTR=1
+fi
+
disable_lp $MOD_LIVEPATCH
unload_lp $MOD_LIVEPATCH
@@ -96,18 +101,19 @@ livepatch: 'test_klp_callbacks_demo': unpatching complete
% rmmod test_klp_callbacks_demo"
fi
-start_test "sysfs test replace enabled"
+if [[ "$HAS_REPLACE_ATTR" == "1" ]]; then
+ start_test "sysfs test replace enabled"
-MOD_LIVEPATCH=test_klp_atomic_replace
-load_lp $MOD_LIVEPATCH replace=1
+ MOD_LIVEPATCH=test_klp_atomic_replace
+ load_lp $MOD_LIVEPATCH replace=1
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
-check_sysfs_value "$MOD_LIVEPATCH" "replace" "1"
+ check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
+ check_sysfs_value "$MOD_LIVEPATCH" "replace" "1"
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
+ disable_lp $MOD_LIVEPATCH
+ unload_lp $MOD_LIVEPATCH
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=1
+ check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=1
livepatch: enabling patch '$MOD_LIVEPATCH'
livepatch: '$MOD_LIVEPATCH': initializing patching transition
livepatch: '$MOD_LIVEPATCH': starting patching transition
@@ -120,17 +126,17 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
livepatch: '$MOD_LIVEPATCH': unpatching complete
% rmmod $MOD_LIVEPATCH"
-start_test "sysfs test replace disabled"
+ start_test "sysfs test replace disabled"
-load_lp $MOD_LIVEPATCH replace=0
+ load_lp $MOD_LIVEPATCH replace=0
-check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
-check_sysfs_value "$MOD_LIVEPATCH" "replace" "0"
+ check_sysfs_rights "$MOD_LIVEPATCH" "replace" "-r--r--r--"
+ check_sysfs_value "$MOD_LIVEPATCH" "replace" "0"
-disable_lp $MOD_LIVEPATCH
-unload_lp $MOD_LIVEPATCH
+ disable_lp $MOD_LIVEPATCH
+ unload_lp $MOD_LIVEPATCH
-check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=0
+ check_result "% insmod test_modules/$MOD_LIVEPATCH.ko replace=0
livepatch: enabling patch '$MOD_LIVEPATCH'
livepatch: '$MOD_LIVEPATCH': initializing patching transition
livepatch: '$MOD_LIVEPATCH': starting patching transition
@@ -142,6 +148,7 @@ livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
livepatch: '$MOD_LIVEPATCH': unpatching complete
% rmmod $MOD_LIVEPATCH"
+fi
start_test "sysfs test stack_order value"
--
2.54.0
^ permalink raw reply related
* [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
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