* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Petr Pavlu @ 2026-03-24 16:00 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <20260324125304.GA15972@wp.pl>
On 3/24/26 1:53 PM, Stanislaw Gruszka wrote:
> Hi,
>
> On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
>> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
>>> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
>>> over the entire symtab when resolving an address. The number of symbols
>>> in module symtabs has grown over the years, largely due to additional
>>> metadata in non-standard sections, making this lookup very slow.
>>>
>>> Improve this by separating function symbols during module load, placing
>>> them at the beginning of the symtab, sorting them by address, and using
>>> binary search when resolving addresses in module text.
>>
>> Doesn't considering only function symbols break the expected behavior
>> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
>> able to see all symbols in a module? The module loader should be remain
>> consistent with the main kallsyms code regarding which symbols can be
>> looked up.
>
> We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and
> module symbol lookup, independent of this patch. find_kallsyms_symbol()
> restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
> it cannot resolve data or rodata symbols.
My understanding is that find_kallsyms_symbol() can identify all symbols
in a module by their addresses. However, the issue I see with
MOD_TEXT/MOD_INIT_TEXT is that the function may incorrectly calculate
the size of symbols that are not within these ranges, which is a bug
that should be fixed.
A test using kdb confirms that non-text symbols can be found by their
addresses. The following shows the current behavior with 7.0-rc5 when
printing a module parameter in mlx4_en:
[1]kdb> mds __param_arr_num_vfs
0xffffffffc1209f20 0000000100000003 ........
0xffffffffc1209f28 ffffffffc0fbf07c [mlx4_core]num_vfs_argc
0xffffffffc1209f30 ffffffff8844bba0 param_ops_byte
0xffffffffc1209f38 ffffffffc0fbf080 [mlx4_core]num_vfs
0xffffffffc1209f40 000000785f69736d msi_x...
0xffffffffc1209f48 656c5f6775626564 debug_le
0xffffffffc1209f50 00000000006c6576 vel.....
0xffffffffc1209f58 0000000000000000 ........
.. and the behavior with the proposed patch:
[1]kdb> mds __param_arr_num_vfs
0xffffffffc1077f20 0000000100000003 ........
0xffffffffc1077f28 ffffffffc104707c |p......
0xffffffffc1077f30 ffffffffb4a4bba0 param_ops_byte
0xffffffffc1077f38 ffffffffc1047080 .p......
0xffffffffc1077f40 000000785f69736d msi_x...
0xffffffffc1077f48 656c5f6775626564 debug_le
0xffffffffc1077f50 00000000006c6576 vel.....
0xffffffffc1077f58 0000000000000000 ........
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-03-25 8:26 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: <282574df-7689-4677-929b-b844e7201bd5@suse.com>
On Tue, Mar 24, 2026 at 05:00:19PM +0100, Petr Pavlu wrote:
> On 3/24/26 1:53 PM, Stanislaw Gruszka wrote:
> > Hi,
> >
> > On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
> >> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
> >>> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
> >>> over the entire symtab when resolving an address. The number of symbols
> >>> in module symtabs has grown over the years, largely due to additional
> >>> metadata in non-standard sections, making this lookup very slow.
> >>>
> >>> Improve this by separating function symbols during module load, placing
> >>> them at the beginning of the symtab, sorting them by address, and using
> >>> binary search when resolving addresses in module text.
> >>
> >> Doesn't considering only function symbols break the expected behavior
> >> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
> >> able to see all symbols in a module? The module loader should be remain
> >> consistent with the main kallsyms code regarding which symbols can be
> >> looked up.
> >
> > We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and
> > module symbol lookup, independent of this patch. find_kallsyms_symbol()
> > restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
> > it cannot resolve data or rodata symbols.
>
> My understanding is that find_kallsyms_symbol() can identify all symbols
> in a module by their addresses. However, the issue I see with
> MOD_TEXT/MOD_INIT_TEXT is that the function may incorrectly calculate
> the size of symbols that are not within these ranges, which is a bug
> that should be fixed.
You are right, I misinterpreted the code:
if (within_module_init(addr, mod))
mod_mem = &mod->mem[MOD_INIT_TEXT];
else
mod_mem = &mod->mem[MOD_TEXT];
nextval = (unsigned long)mod_mem->base + mod_mem->size;
bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
For best = 0, bestval is also 0 as it comes from the ELF null symbol.
> A test using kdb confirms that non-text symbols can be found by their
> addresses. The following shows the current behavior with 7.0-rc5 when
> printing a module parameter in mlx4_en:
>
> [1]kdb> mds __param_arr_num_vfs
> 0xffffffffc1209f20 0000000100000003 ........
> 0xffffffffc1209f28 ffffffffc0fbf07c [mlx4_core]num_vfs_argc
> 0xffffffffc1209f30 ffffffff8844bba0 param_ops_byte
> 0xffffffffc1209f38 ffffffffc0fbf080 [mlx4_core]num_vfs
> 0xffffffffc1209f40 000000785f69736d msi_x...
> 0xffffffffc1209f48 656c5f6775626564 debug_le
> 0xffffffffc1209f50 00000000006c6576 vel.....
> 0xffffffffc1209f58 0000000000000000 ........
>
> .. and the behavior with the proposed patch:
>
> [1]kdb> mds __param_arr_num_vfs
> 0xffffffffc1077f20 0000000100000003 ........
> 0xffffffffc1077f28 ffffffffc104707c |p......
> 0xffffffffc1077f30 ffffffffb4a4bba0 param_ops_byte
> 0xffffffffc1077f38 ffffffffc1047080 .p......
> 0xffffffffc1077f40 000000785f69736d msi_x...
> 0xffffffffc1077f48 656c5f6775626564 debug_le
> 0xffffffffc1077f50 00000000006c6576 vel.....
> 0xffffffffc1077f58 0000000000000000 ........
Thanks for testing and pointing this out. Patch indeed breaks
the CONFIG_KALLSYMS_ALL case.
I think, possible fix would be to track the relevant sections in
__layout_sections() and use defined symbols from those sections,
instead of just function symbols.
Regards
Stanislaw
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Miroslav Benes @ 2026-03-25 8:45 UTC (permalink / raw)
To: Joe Lawrence
Cc: Miroslav Benes, Pablo Hugen, live-patching, linux-kselftest,
linux-kernel, jpoimboe, jikos, pmladek, shuah
In-Reply-To: <acKje1XMQMQQYBIL@redhat.com>
> > A nit but is 'noinline' keyword needed here? proc_create_single() below
> > takes a function pointer so hopefully test_klp_mod_target_show() stays
> > even without it?
>
> No strong preference either way. I won't fault a livepatch developer
> for being paranoid w/respect to the compiler :D
True :)
Either way
Acked-by: Miroslav Benes <mbenes@suse.cz>
^ permalink raw reply
* Re: [PATCH] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-03-25 10:02 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: <20260325082648.GA18968@wp.pl>
On Wed, Mar 25, 2026 at 09:26:56AM +0100, Stanislaw Gruszka wrote:
> On Tue, Mar 24, 2026 at 05:00:19PM +0100, Petr Pavlu wrote:
> > On 3/24/26 1:53 PM, Stanislaw Gruszka wrote:
> > > Hi,
> > >
> > > On Mon, Mar 23, 2026 at 02:06:43PM +0100, Petr Pavlu wrote:
> > >> On 3/17/26 12:04 PM, Stanislaw Gruszka wrote:
> > >>> Module symbol lookup via find_kallsyms_symbol() performs a linear scan
> > >>> over the entire symtab when resolving an address. The number of symbols
> > >>> in module symtabs has grown over the years, largely due to additional
> > >>> metadata in non-standard sections, making this lookup very slow.
> > >>>
> > >>> Improve this by separating function symbols during module load, placing
> > >>> them at the beginning of the symtab, sorting them by address, and using
> > >>> binary search when resolving addresses in module text.
> > >>
> > >> Doesn't considering only function symbols break the expected behavior
> > >> with CONFIG_KALLSYMS_ALL=y. For instance, when using kdb, is it still
> > >> able to see all symbols in a module? The module loader should be remain
> > >> consistent with the main kallsyms code regarding which symbols can be
> > >> looked up.
> > >
> > > We already have a CONFIG_KALLSYMS_ALL=y inconsistency between kernel and
> > > module symbol lookup, independent of this patch. find_kallsyms_symbol()
> > > restricts the search to MOD_TEXT (or MOD_INIT_TEXT) address ranges, so
> > > it cannot resolve data or rodata symbols.
> >
> > My understanding is that find_kallsyms_symbol() can identify all symbols
> > in a module by their addresses. However, the issue I see with
> > MOD_TEXT/MOD_INIT_TEXT is that the function may incorrectly calculate
> > the size of symbols that are not within these ranges, which is a bug
> > that should be fixed.
>
> You are right, I misinterpreted the code:
>
> if (within_module_init(addr, mod))
> mod_mem = &mod->mem[MOD_INIT_TEXT];
> else
> mod_mem = &mod->mem[MOD_TEXT];
>
> nextval = (unsigned long)mod_mem->base + mod_mem->size;
>
> bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
>
> For best = 0, bestval is also 0 as it comes from the ELF null symbol.
>
> > A test using kdb confirms that non-text symbols can be found by their
> > addresses. The following shows the current behavior with 7.0-rc5 when
> > printing a module parameter in mlx4_en:
> >
> > [1]kdb> mds __param_arr_num_vfs
> > 0xffffffffc1209f20 0000000100000003 ........
> > 0xffffffffc1209f28 ffffffffc0fbf07c [mlx4_core]num_vfs_argc
> > 0xffffffffc1209f30 ffffffff8844bba0 param_ops_byte
> > 0xffffffffc1209f38 ffffffffc0fbf080 [mlx4_core]num_vfs
> > 0xffffffffc1209f40 000000785f69736d msi_x...
> > 0xffffffffc1209f48 656c5f6775626564 debug_le
> > 0xffffffffc1209f50 00000000006c6576 vel.....
> > 0xffffffffc1209f58 0000000000000000 ........
> >
> > .. and the behavior with the proposed patch:
> >
> > [1]kdb> mds __param_arr_num_vfs
> > 0xffffffffc1077f20 0000000100000003 ........
> > 0xffffffffc1077f28 ffffffffc104707c |p......
> > 0xffffffffc1077f30 ffffffffb4a4bba0 param_ops_byte
> > 0xffffffffc1077f38 ffffffffc1047080 .p......
> > 0xffffffffc1077f40 000000785f69736d msi_x...
> > 0xffffffffc1077f48 656c5f6775626564 debug_le
> > 0xffffffffc1077f50 00000000006c6576 vel.....
> > 0xffffffffc1077f58 0000000000000000 ........
>
> Thanks for testing and pointing this out. Patch indeed breaks
> the CONFIG_KALLSYMS_ALL case.
>
> I think, possible fix would be to track the relevant sections in
> __layout_sections() and use defined symbols from those sections,
> instead of just function symbols.
I considered sorting data symbols as well, but this is nontrivial,
it is difficult to reliably distinguish real data sections from metadata
sections containing symbols we do not want to include.
An alternative approach is to check the module memory type and fall back to
a linear search for ranges other than MOD_TEXT. This approach would also
fix the incorrect nextval/size problem.
Regards
Stanislaw
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Petr Mladek @ 2026-03-26 14:34 UTC (permalink / raw)
To: Pablo Hugen
Cc: live-patching, linux-kselftest, linux-kernel, jpoimboe, jikos,
mbenes, joe.lawrence, shuah
In-Reply-To: <20260320201135.1203992-1-phugen@redhat.com>
On Fri 2026-03-20 17:11:17, Pablo Hugen wrote:
> From: Pablo Alessandro Santos Hugen <phugen@redhat.com>
>
> Add a target module and livepatch pair that verify module function
> patching via a proc entry. Two test cases cover both the
> klp_enable_patch path (target loaded before livepatch) and the
> klp_module_coming path (livepatch loaded before target).
First, thanks for the test.
Second, I am a bit biased because I am working on a patchset which would
obsolete this patch, see
https://lore.kernel.org/all/20250115082431.5550-1-pmladek@suse.com/
That said, I have sent an RFC a year ago. I worked on v1 when time
permitted but it is still not ready. And it might take another
many months or year to finish it.
Your test might be perfectly fine in the meantime. Just see few
notes below.
> --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> @@ -8,6 +8,8 @@ MOD_LIVEPATCH1=test_klp_livepatch
> MOD_LIVEPATCH2=test_klp_syscall
> MOD_LIVEPATCH3=test_klp_callbacks_demo
> MOD_REPLACE=test_klp_atomic_replace
> +MOD_TARGET=test_klp_mod_target
> +MOD_TARGET_PATCH=test_klp_mod_patch
>
> setup_config
>
> @@ -196,4 +198,102 @@ livepatch: '$MOD_REPLACE': unpatching complete
> % rmmod $MOD_REPLACE"
>
>
> +# - load a target module that provides /proc/test_klp_mod_target with
> +# original output
> +# - load a livepatch that patches the target module's show function
> +# - verify the proc entry returns livepatched output
> +# - disable and unload the livepatch
> +# - verify the proc entry returns original output again
> +# - unload the target module
> +
> +start_test "module function patching"
> +
> +load_mod $MOD_TARGET
> +
> +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> + echo -e "FAIL\n\n"
> + die "livepatch kselftest(s) failed"
> +fi
This code is repeated several times. It might be worth creating a
helper function in tools/testing/selftests/livepatch/functions.sh.
> +load_lp $MOD_TARGET_PATCH
> +
> +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
> + echo -e "FAIL\n\n"
> + die "livepatch kselftest(s) failed"
> +fi
When I was working on the above mentioned patchset, I realized that
"die" in the middle of the test was not practical because it
did not do any clean up. As a result, "make run_tests"
continued with other tests but they typically failed as well.
And I had to manually remove the test modules to be able to
try "fixed" tests again.
I thought about two solutions:
1. Remember loaded modules and try to remove them in a clean up code.
2. Report the failure into the kernel log but keep the test
running so that they calls the disable_lp/unload_lp/unload_mod
functions. The test will do the clean up and will fail
later in check_result().
While the 1st approach might be easier in the end, I choose
the 2nd approach in my RFC, see below.
> +disable_lp $MOD_TARGET_PATCH
> +unload_lp $MOD_TARGET_PATCH
> +
> +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> + echo -e "FAIL\n\n"
> + die "livepatch kselftest(s) failed"
> +fi
> +
> +unload_mod $MOD_TARGET
> +
> +check_result "% insmod test_modules/$MOD_TARGET.ko
> +$MOD_TARGET: test_klp_mod_target_init
> +% insmod test_modules/$MOD_TARGET_PATCH.ko
Note that the existing helper functions log the userspace commands
in the kernel log. It helps to understand the kernel logs.
In my RFC, I created a helper module which implemented a person
(speaker) which would come on the stage and welcome the audience.
I am not sure if it was a good idea. But it became a bit confusing
when everything (module name, sysfs interface, function name, message)
included the same strings like (livepatch, callback, shadow_var).
Anyway, my tests produced messages like these:
+% cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/welcome
+$MOD_TARGET: speaker_welcome: Hello, World!
, see https://lore.kernel.org/all/20250115082431.5550-9-pmladek@suse.com/
There were even tests which blocked the transition. They tested shadow
variables which added an applause to the message. They did something like:
<paste>
All four callbacks are used as follows:
+ pre_patch() allocates a shadow variable with a string and fills
it with "[]".
+ post_patch() fills the string with "[APPLAUSE]".
+ pre_unpatch() reverts the string back to "[]".
+ post_unpatch() releases the shadow variable.
The welcome message printed by the livepatched function allows us to
distinguish between the transition and the completed transition.
Specifically, the speaker's welcome message appears as:
+ Not patched system: "Hello, World!"
+ Transition (unpatched task): "[] Hello, World!"
+ Transition (patched task): "[] Ladies and gentlemen, ..."
+ Patched system: "[APPLAUSE] Ladies and gentlemen, ..."
</paste>
, see https://lore.kernel.org/all/20250115082431.5550-11-pmladek@suse.com/
Sigh, I have done many changes in the tests for v1. But they still
need some love (and rebasing) for sending.
> +livepatch: enabling patch '$MOD_TARGET_PATCH'
> +livepatch: '$MOD_TARGET_PATCH': initializing patching transition
> +livepatch: '$MOD_TARGET_PATCH': starting patching transition
> +livepatch: '$MOD_TARGET_PATCH': completing patching transition
> +livepatch: '$MOD_TARGET_PATCH': patching complete
> +% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
> +livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
> +livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
> +livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
> +livepatch: '$MOD_TARGET_PATCH': unpatching complete
> +% rmmod $MOD_TARGET_PATCH
> +% rmmod $MOD_TARGET
> +$MOD_TARGET: test_klp_mod_target_exit"
Summary:
IMHO, this patch is perfectly fine as is if we accept that it will get
eventually obsoleted by my patchset (hopefully in a year or two).
On the other hand, this patch would deserve some clean up,
(helper functions, don't die in the middle of the test) if
you planned to work on more tests. It would help to maintain
the tests.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Joe Lawrence @ 2026-03-26 20:41 UTC (permalink / raw)
To: Petr Mladek
Cc: Pablo Hugen, live-patching, linux-kselftest, linux-kernel,
jpoimboe, jikos, mbenes, shuah
In-Reply-To: <acVD_NPu4JVRoaVK@pathway.suse.cz>
On Thu, Mar 26, 2026 at 03:34:36PM +0100, Petr Mladek wrote:
> On Fri 2026-03-20 17:11:17, Pablo Hugen wrote:
> > From: Pablo Alessandro Santos Hugen <phugen@redhat.com>
> >
> > Add a target module and livepatch pair that verify module function
> > patching via a proc entry. Two test cases cover both the
> > klp_enable_patch path (target loaded before livepatch) and the
> > klp_module_coming path (livepatch loaded before target).
>
> First, thanks for the test.
>
> Second, I am a bit biased because I am working on a patchset which would
> obsolete this patch, see
> https://lore.kernel.org/all/20250115082431.5550-1-pmladek@suse.com/
>
> That said, I have sent an RFC a year ago. I worked on v1 when time
> permitted but it is still not ready. And it might take another
> many months or year to finish it.
>
Hi Petr,
I remember that patch as a "grand, unified" theory for livepatching API
and look forward to the next iteration when you find the time.
> Your test might be perfectly fine in the meantime. Just see few
> notes below.
>
> > --- a/tools/testing/selftests/livepatch/test-livepatch.sh
> > +++ b/tools/testing/selftests/livepatch/test-livepatch.sh
> > @@ -8,6 +8,8 @@ MOD_LIVEPATCH1=test_klp_livepatch
> > MOD_LIVEPATCH2=test_klp_syscall
> > MOD_LIVEPATCH3=test_klp_callbacks_demo
> > MOD_REPLACE=test_klp_atomic_replace
> > +MOD_TARGET=test_klp_mod_target
> > +MOD_TARGET_PATCH=test_klp_mod_patch
> >
> > setup_config
> >
> > @@ -196,4 +198,102 @@ livepatch: '$MOD_REPLACE': unpatching complete
> > % rmmod $MOD_REPLACE"
> >
> >
> > +# - load a target module that provides /proc/test_klp_mod_target with
> > +# original output
> > +# - load a livepatch that patches the target module's show function
> > +# - verify the proc entry returns livepatched output
> > +# - disable and unload the livepatch
> > +# - verify the proc entry returns original output again
> > +# - unload the target module
> > +
> > +start_test "module function patching"
> > +
> > +load_mod $MOD_TARGET
> > +
> > +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> > + echo -e "FAIL\n\n"
> > + die "livepatch kselftest(s) failed"
> > +fi
>
> This code is repeated several times. It might be worth creating a
> helper function in tools/testing/selftests/livepatch/functions.sh.
>
> > +load_lp $MOD_TARGET_PATCH
> > +
> > +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
> > + echo -e "FAIL\n\n"
> > + die "livepatch kselftest(s) failed"
> > +fi
>
> When I was working on the above mentioned patchset, I realized that
> "die" in the middle of the test was not practical because it
> did not do any clean up.
IIRC it was an original design intention so that if you were to run the
test script directly, the system would be left in that state for
debugging. That said ...
> As a result, "make run_tests"
> continued with other tests but they typically failed as well.
> And I had to manually remove the test modules to be able to
> try "fixed" tests again.
... D'oh, that's pretty annoying behavior to wade through, like triaging
compiler failures (i.e. the Nth error is probably just a result of the
first N-1 error(s).)
Since most people probably run via the run_tests target, my previous
preference for a debuggable state probably isn't realistic anymore as
the test car continues careening down the cliff, accumulating new,
interesting damage with each subsequent test failure.
>
> I thought about two solutions:
>
> 1. Remember loaded modules and try to remove them in a clean up code.
>
At the time of creating the selftests, I remember resisting this effort,
afraid of the Pareto Principle and never getting that last 20% correct
(is the livepatch transition stuck, broken, how I can cleanly unwind
from a test gone rogue, etc.)
> 2. Report the failure into the kernel log but keep the test
> running so that they calls the disable_lp/unload_lp/unload_mod
> functions. The test will do the clean up and will fail
> later in check_result().
>
>
> While the 1st approach might be easier in the end, I choose
> the 2nd approach in my RFC, see below.
>
>
> > +disable_lp $MOD_TARGET_PATCH
> > +unload_lp $MOD_TARGET_PATCH
> > +
> > +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> > + echo -e "FAIL\n\n"
> > + die "livepatch kselftest(s) failed"
> > +fi
> > +
> > +unload_mod $MOD_TARGET
> > +
> > +check_result "% insmod test_modules/$MOD_TARGET.ko
> > +$MOD_TARGET: test_klp_mod_target_init
> > +% insmod test_modules/$MOD_TARGET_PATCH.ko
>
So following this technique, all the other tests with command sequences
would need to be re-written as '&&' chains, e.g. the "patch getpid
syscall while being heavily hammered" one like:
pid_list=$(echo "${pids[@]}" | tr ' ' ',') && \
load_lp $MOD_SYSCALL klp_pids=$pid_list && \
loop_until "grep -q '^0$' $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids" && \
log "$MOD_SYSCALL: Remaining not livepatched processes: $(cat $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids)"
so that we only continue down a particular test for as long as it's
successful, then the cleanup code is unconditional:
pending_pids=$(cat $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids)
log "$MOD_SYSCALL: Remaining not livepatched processes: $pending_pids"
for pid in ${pids[@]}; do
kill $pid || true
done
disable_lp $MOD_SYSCALL
unload_lp $MOD_SYSCALL
check_result <- flags a problem
Yeah, may be that's not so bad. The functions.sh helpers may need to be
hardened a little (can they cancel / bust a transition? it's been a
while since I've looked.)
Or maybe ... ugh, bash is not a programming language ... each test is
split into its own script, the die calls can remain as they are, but we
move the cleanup logic into a trap EXIT handler so it always runs?
Or test-code and test-cleanup-code are split into their own functions,
so tests can exit early, but their cleanup is always called?
(Just brainstorming here.)
> Note that the existing helper functions log the userspace commands
> in the kernel log. It helps to understand the kernel logs.
>
> In my RFC, I created a helper module which implemented a person
> (speaker) which would come on the stage and welcome the audience.
> I am not sure if it was a good idea. But it became a bit confusing
> when everything (module name, sysfs interface, function name, message)
> included the same strings like (livepatch, callback, shadow_var).
>
> Anyway, my tests produced messages like these:
>
> +% cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/welcome
> +$MOD_TARGET: speaker_welcome: Hello, World!
>
> , see https://lore.kernel.org/all/20250115082431.5550-9-pmladek@suse.com/
>
>
> There were even tests which blocked the transition. They tested shadow
> variables which added an applause to the message. They did something like:
>
> <paste>
> All four callbacks are used as follows:
>
> + pre_patch() allocates a shadow variable with a string and fills
> it with "[]".
> + post_patch() fills the string with "[APPLAUSE]".
> + pre_unpatch() reverts the string back to "[]".
> + post_unpatch() releases the shadow variable.
>
> The welcome message printed by the livepatched function allows us to
> distinguish between the transition and the completed transition.
> Specifically, the speaker's welcome message appears as:
>
> + Not patched system: "Hello, World!"
> + Transition (unpatched task): "[] Hello, World!"
> + Transition (patched task): "[] Ladies and gentlemen, ..."
> + Patched system: "[APPLAUSE] Ladies and gentlemen, ..."
> </paste>
>
> , see https://lore.kernel.org/all/20250115082431.5550-11-pmladek@suse.com/
>
> Sigh, I have done many changes in the tests for v1. But they still
> need some love (and rebasing) for sending.
>
If these can be pulled out independently from the v1 patch, perhaps
Pablo would want to hack on that in a follow up series?
> > +livepatch: enabling patch '$MOD_TARGET_PATCH'
> > +livepatch: '$MOD_TARGET_PATCH': initializing patching transition
> > +livepatch: '$MOD_TARGET_PATCH': starting patching transition
> > +livepatch: '$MOD_TARGET_PATCH': completing patching transition
> > +livepatch: '$MOD_TARGET_PATCH': patching complete
> > +% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
> > +livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
> > +livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
> > +livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
> > +livepatch: '$MOD_TARGET_PATCH': unpatching complete
> > +% rmmod $MOD_TARGET_PATCH
> > +% rmmod $MOD_TARGET
> > +$MOD_TARGET: test_klp_mod_target_exit"
>
> Summary:
>
> IMHO, this patch is perfectly fine as is if we accept that it will get
> eventually obsoleted by my patchset (hopefully in a year or two).
>
> On the other hand, this patch would deserve some clean up,
> (helper functions, don't die in the middle of the test) if
> you planned to work on more tests. It would help to maintain
> the tests.
>
Right, I think this was a good intro patch for Pablo and that the
revised execution flow would be a great follow on series, if he is
interested. How about that?
Regards,
--
Joe
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Miroslav Benes @ 2026-03-27 10:46 UTC (permalink / raw)
To: Joe Lawrence
Cc: Petr Mladek, Pablo Hugen, live-patching, linux-kselftest,
linux-kernel, jpoimboe, jikos, shuah
In-Reply-To: <acWZ3r2CoSDy_NLf@redhat.com>
> > > +disable_lp $MOD_TARGET_PATCH
> > > +unload_lp $MOD_TARGET_PATCH
> > > +
> > > +if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
> > > + echo -e "FAIL\n\n"
> > > + die "livepatch kselftest(s) failed"
> > > +fi
> > > +
> > > +unload_mod $MOD_TARGET
> > > +
> > > +check_result "% insmod test_modules/$MOD_TARGET.ko
> > > +$MOD_TARGET: test_klp_mod_target_init
> > > +% insmod test_modules/$MOD_TARGET_PATCH.ko
> >
>
> So following this technique, all the other tests with command sequences
> would need to be re-written as '&&' chains, e.g. the "patch getpid
> syscall while being heavily hammered" one like:
>
> pid_list=$(echo "${pids[@]}" | tr ' ' ',') && \
> load_lp $MOD_SYSCALL klp_pids=$pid_list && \
> loop_until "grep -q '^0$' $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids" && \
> log "$MOD_SYSCALL: Remaining not livepatched processes: $(cat $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids)"
>
> so that we only continue down a particular test for as long as it's
> successful, then the cleanup code is unconditional:
>
> pending_pids=$(cat $SYSFS_KERNEL_DIR/$MOD_SYSCALL/npids)
> log "$MOD_SYSCALL: Remaining not livepatched processes: $pending_pids"
>
> for pid in ${pids[@]}; do
> kill $pid || true
> done
>
> disable_lp $MOD_SYSCALL
> unload_lp $MOD_SYSCALL
>
> check_result <- flags a problem
>
> Yeah, may be that's not so bad. The functions.sh helpers may need to be
> hardened a little (can they cancel / bust a transition? it's been a
> while since I've looked.)
>
> Or maybe ... ugh, bash is not a programming language ... each test is
> split into its own script, the die calls can remain as they are, but we
> move the cleanup logic into a trap EXIT handler so it always runs?
We use this technique in OOT https://github.com/SUSE/qa_test_klp/ tests
(slowly being upstreamed). See
https://github.com/SUSE/qa_test_klp/blob/master/klp_tc_functions.sh.
Mainly klp_tc_init(), klp_tc_exit() and klp_tc_abort(). Different tests
then use what you proposed above... caching pids and modules to clean up.
Miroslav
^ permalink raw reply
* [PATCH v2 1/2] module/kallsyms: fix nextval for data symbol lookup
From: Stanislaw Gruszka @ 2026-03-27 11:00 UTC (permalink / raw)
To: linux-modules, Sami Tolvanen, Luis Chamberlain, Petr Pavlu
Cc: linux-kernel, linux-trace-kernel, live-patching, Daniel Gomez,
Aaron Tomlin, Steven Rostedt, Masami Hiramatsu, Jordan Rome,
Viktor Malik
The symbol lookup code assumes the queried address resides in either
MOD_TEXT or MOD_INIT_TEXT. This breaks for addresses in other module
memory regions (e.g. rodata or data), resulting in incorrect upper
bounds and wrong symbol size.
Select the module memory region the address belongs to instead of
hardcoding text sections. Also initialize the lower bound to the start
of that region, as searching from address 0 is unnecessary.
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
---
v1 -> v2: new patch.
kernel/module/kallsyms.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 0fc11e45df9b..f23126d804b2 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -258,17 +258,25 @@ static const char *find_kallsyms_symbol(struct module *mod,
unsigned int i, best = 0;
unsigned long nextval, bestval;
struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
- struct module_memory *mod_mem;
+ struct module_memory *mod_mem = NULL;
- /* At worse, next value is at end of module */
- if (within_module_init(addr, mod))
- mod_mem = &mod->mem[MOD_INIT_TEXT];
- else
- mod_mem = &mod->mem[MOD_TEXT];
+ for_each_mod_mem_type(type) {
+#ifndef CONFIG_KALLSYMS_ALL
+ if (!mod_mem_type_is_text(type))
+ continue;
+#endif
+ if (within_module_mem_type(addr, mod, type)) {
+ mod_mem = &mod->mem[type];
+ break;
+ }
+ }
- nextval = (unsigned long)mod_mem->base + mod_mem->size;
+ if (!mod_mem)
+ return NULL;
- bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
+ /* Initialize bounds within memory region the address belongs to. */
+ nextval = (unsigned long)mod_mem->base + mod_mem->size;
+ bestval = (unsigned long)mod_mem->base - 1;
/*
* Scan for closest preceding symbol, and next symbol. (ELF
--
2.50.1
^ permalink raw reply related
* [PATCH v2 2/2] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-03-27 11:00 UTC (permalink / raw)
To: linux-modules, Sami Tolvanen, Luis Chamberlain, Petr Pavlu
Cc: linux-kernel, linux-trace-kernel, live-patching, Daniel Gomez,
Aaron Tomlin, Steven Rostedt, Masami Hiramatsu, Jordan Rome,
Viktor Malik
In-Reply-To: <20260327110005.16499-1-stf_xl@wp.pl>
Module symbol lookup via find_kallsyms_symbol() performs a linear scan
over the entire symtab when resolving an address. The number of symbols
in module symtabs has grown over the years, largely due to additional
metadata in non-standard sections, making this lookup very slow.
Improve this by separating function symbols during module load, placing
them at the beginning of the symtab, sorting them by address, and using
binary search when resolving addresses in module text.
This also should improve times for linear symbol name lookups, as valid
function symbols are now located at the beginning of the symtab.
The cost of sorting is small relative to module load time. In repeated
module load tests [1], depending on .config options, this change
increases load time between 2% and 4%. With cold caches, the difference
is not measurable, as memory access latency dominates.
The sorting theoretically could be done in compile time, but much more
complicated as we would have to simulate kernel addresses resolution
for symbols, and then correct relocation entries. That would be risky
if get out of sync.
The improvement can be observed when listing ftrace filter functions.
Before:
root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
74908
real 0m1.315s
user 0m0.000s
sys 0m1.312s
After:
root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
74911
real 0m0.167s
user 0m0.004s
sys 0m0.175s
(there are three more symbols introduced by the patch)
For livepatch modules, the symtab layout is preserved and the existing
linear search is used. For this case, it should be possible to keep
the original ELF symtab instead of copying it 1:1, but that is outside
the scope of this patch.
Link: https://gist.github.com/sgruszka/09f3fb1dad53a97b1aad96e1927ab117 [1]
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
---
v1 -> v2:
- fix searching data symbols for CONFIG_KALLSYMS_ALL
- use kallsyms_symbol_value() in elf_sym_cmp()
include/linux/module.h | 1 +
kernel/module/internal.h | 1 +
kernel/module/kallsyms.c | 171 +++++++++++++++++++++++++++++----------
3 files changed, 130 insertions(+), 43 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index ac254525014c..67c053afa882 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -379,6 +379,7 @@ struct module_memory {
struct mod_kallsyms {
Elf_Sym *symtab;
unsigned int num_symtab;
+ unsigned int num_func_syms;
char *strtab;
char *typetab;
};
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 618202578b42..6a4d498619b1 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -73,6 +73,7 @@ struct load_info {
bool sig_ok;
#ifdef CONFIG_KALLSYMS
unsigned long mod_kallsyms_init_off;
+ unsigned long num_func_syms;
#endif
#ifdef CONFIG_MODULE_DECOMPRESS
#ifdef CONFIG_MODULE_STATS
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;
+ } 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;
+}
+
/*
* 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;
@@ -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);
+
/* 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;
+ else
+ search = search_kallsyms_symbol;
+
mod_mem = &mod->mem[type];
break;
}
@@ -278,33 +389,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
nextval = (unsigned long)mod_mem->base + mod_mem->size;
bestval = (unsigned long)mod_mem->base - 1;
- /*
- * Scan for closest preceding symbol, and next symbol. (ELF
- * starts real symbols at 1).
- */
- for (i = 1; 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;
- }
-
+ best = search(kallsyms, addr, &bestval, &nextval);
if (!best)
return NULL;
--
2.50.1
^ permalink raw reply related
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Marcos Paulo de Souza @ 2026-03-27 13:16 UTC (permalink / raw)
To: Joe Lawrence
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <abhjYtyveer4niGM@redhat.com>
On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> wrote:
> > Instead of checking if the architecture running the test was
> > powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > index dd802783ea849..c01a586866304 100644
> > ---
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if
> the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might
> be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
Sorry for the late reply...
Well, so far the test always run fine for us, so I never looked why ppc
didn't have ARCH_HAS_SYSCALL_WRAPPER TBH... I can add some information
about it, sure.
>
> Thanks,
> --
> Joe
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Marcos Paulo de Souza @ 2026-03-27 13:24 UTC (permalink / raw)
To: Miroslav Benes
Cc: Joe Lawrence, Josh Poimboeuf, Jiri Kosina, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <alpine.LSU.2.21.2603201136401.12616@pobox.suse.cz>
On Fri, 2026-03-20 at 11:45 +0100, Miroslav Benes wrote:
> > > So I would perhaps prefer to stay with the logic that defines
> > > FN_PREFIX
> > > per architecture and has also #else branch for the rest. And more
> > > comments
> > > never hurt.
> >
> > Agreed.
>
> Hm, so I thought about a bit more and I very likely misunderstood the
> motivation behind the patch. I will speculate and correct me if I am
> wrong, please. The idea behind the whole patch set is to make the
> selftests run on older kernels which I think is something we should
> support. The issue is that old kernels (like mentioned 4.12) do not
> have
> syscall wrappers at all. getpid() syscall is just plain old
> sys_getpid
> there and not the current __x64_sys_getpid on x86. The patch fixes it
> by checking CONFIG_ARCH_HAS_SYSCALL_WRAPPER and defining FN_PREFIX
> accordingly.
Exactly. The definition was added on
commit 1bd21c6c21e848996339508d3ffb106d505256a8
Author: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Thu Apr 5 11:53:01 2018 +0200
syscalls/core: Introduce CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y
>
> So, if this is correct, I think it should be done differently. We
> should
> have something like syscall_wrapper.h which would define FN_PREFIX
> for
> the supported architectures and different kernel versions since the
> wrappers may have changed a couple of times during the history. In
> that
> case there could then be an #else branch which might just error out
> with
> the message to add proper syscall wrapper naming.
Well, it seems too much for a simple test to me, but I can do that, no
problem.
>
> The changelog then should explain it because it is not in fact tight
> to
> powerpc.
Makes sense, I'll change it.
>
> What do you think? Am I off again?
I agree with everything, but adding another header file seems a little
too much work for a simple test case, but it's doable. Let me work on
it.
>
> Miroslav
^ permalink raw reply
* Re: [PATCH 3/8] selftests: livepatch: test-kprobe: Check if kprobes can work with livepatches
From: Marcos Paulo de Souza @ 2026-03-27 13:43 UTC (permalink / raw)
To: Petr Mladek
Cc: Joe Lawrence, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <ab0wiEuokqDwq5_v@pathway.suse.cz>
On Fri, 2026-03-20 at 12:33 +0100, Petr Mladek wrote:
> On Thu 2026-03-19 11:35:16, Marcos Paulo de Souza wrote:
> > On Mon, 2026-03-16 at 16:38 -0400, Joe Lawrence wrote:
> > > On Fri, Mar 13, 2026 at 05:58:34PM -0300, Marcos Paulo de Souza
> > > wrote:
> > > > Running the upstream selftests on older kernels can presente
> > > > some
> > > > issues
> > > > regarding features being not present. One of such issues if the
> > > > missing
> > > > capability of having both kprobes and livepatches on the same
> > > > function.
> > > >
> > >
> > > nit picking, but slightly reworded for clarity and spelling:
> > >
> > > Running upstream selftests on older kernels can be problematic
> > > when
> > > features or fixes from newer versions are not present. For
> > > example,
> > > older kernels may lack the capability to support kprobes and
> > > livepatches
> > > on the same function simultaneously.
> >
> > Much better, I'll pick your description for v2.
> >
> > >
> > > > The support was introduced in commit 0bc11ed5ab60c
> > > > ("kprobes: Allow kprobes coexist with livepatch"), which means
> > > > that
> > > > older
> > > > kernels may lack this change.
> > > >
> > > > The lack of this feature can be checked when a kprobe without a
> > > > post_handler is loaded and checking that the enabled_function's
> > > > file
> > > > shows the flag "I". A kernel with the proper support for
> > > > kprobes
> > > > and
> > > > livepatches would presente the flag only when a post_handler is
> > >
> > > nit: s/presente/present
> >
> > > > --- a/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > @@ -16,30 +16,19 @@ setup_config
> > > > # when it uses a post_handler since only one IPMODIFY maybe be
> > > > registered
> > > > # to any given function at a time.
> > > >
> > > > -start_test "livepatch interaction with kprobed function with
> > > > post_handler"
> > > > -
> > > > -echo 1 > "$SYSFS_KPROBES_DIR/enabled"
> > > > -
> > > > -load_mod $MOD_KPROBE has_post_handler=1
> > > > -load_failing_mod $MOD_LIVEPATCH
> > > > -unload_mod $MOD_KPROBE
> > > > -
> > > > -check_result "% insmod test_modules/test_klp_kprobe.ko
> > > > has_post_handler=1
> > > > -% insmod test_modules/$MOD_LIVEPATCH.ko
> > > > -livepatch: enabling patch '$MOD_LIVEPATCH'
> > > > -livepatch: '$MOD_LIVEPATCH': initializing patching transition
> > > > -livepatch: failed to register ftrace handler for function
> > > > 'cmdline_proc_show' (-16)
> > > > -livepatch: failed to patch object 'vmlinux'
> > > > -livepatch: failed to enable patch '$MOD_LIVEPATCH'
> > > > -livepatch: '$MOD_LIVEPATCH': canceling patching transition,
> > > > going
> > > > to unpatch
> > > > -livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> > > > -livepatch: '$MOD_LIVEPATCH': unpatching complete
> > > > -insmod: ERROR: could not insert module
> > > > test_modules/$MOD_LIVEPATCH.ko: Device or resource busy
> > > > -% rmmod test_klp_kprobe"
> > > > -
> > > > start_test "livepatch interaction with kprobed function
> > > > without
> > > > post_handler"
> > > >
> > > > load_mod $MOD_KPROBE has_post_handler=0
> > > > +
> > > > +# Check if commit 0bc11ed5ab60c ("kprobes: Allow kprobes
> > > > coexist
> > > > with livepatch")
> > > > +# is missing, meaning that livepatches and kprobes can't be
> > > > used
> > > > together.
> > > > +# When the commit is missing, kprobes always set IPMODIFY (the
> > > > I
> > > > flag), even
> > > > +# when the post handler is missing.
> > > > +if grep --quiet ") R I"
> > > > "$SYSFS_DEBUG_DIR/tracing/enabled_functions"; then
> > >
> > > Will flags R I always be in this order?
> >
> > seq_printf(m, " (%ld)%s%s%s%s%s",
> > ftrace_rec_count(rec),
> > rec->flags & FTRACE_FL_REGS ? " R" : "
> > ",
> > rec->flags & FTRACE_FL_IPMODIFY ? " I"
> > : "
> > ",
> >
> > So this is safe. I'll add a comment in the patch to explain why
> > this is
> > safe too. Thanks for the comment!
>
> I would personally check also "cmdline_proc_show" to make sure that
> the line is about this function. Something like:
>
> grep --quiet ") "cmdline_proc_show.*([0-9]\+) R"
>
>
> But I am afraid that this approach is not good. It breaks the test.
> It won't longer be able to catch regressions when the kprobe
> sets "FTRACE_FL_IPMODIFY" by mistake again.
>
> We could add a version check. But it would break users who backport
> the fix into older kernels.
>
> IMHO, the best solution would be to keep the test as is.
> Whoever is running the test with older kernels should mark it
> as "failure-expected". The test is pointing out an existing problem
> in the old kernel. IMHO, it should not hide it.
Makes sense, thanks for your review Petr. I'll drop this patch from v2.
>
> Best Regards,
> Petr
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Petr Mladek @ 2026-03-30 13:43 UTC (permalink / raw)
To: Joe Lawrence
Cc: Pablo Hugen, live-patching, linux-kselftest, linux-kernel,
jpoimboe, jikos, mbenes, shuah
In-Reply-To: <acWZ3r2CoSDy_NLf@redhat.com>
On Thu 2026-03-26 16:41:02, Joe Lawrence wrote:
> On Thu, Mar 26, 2026 at 03:34:36PM +0100, Petr Mladek wrote:
> > On Fri 2026-03-20 17:11:17, Pablo Hugen wrote:
> > > From: Pablo Alessandro Santos Hugen <phugen@redhat.com>
> > >
> > > Add a target module and livepatch pair that verify module function
> > > patching via a proc entry. Two test cases cover both the
> > > klp_enable_patch path (target loaded before livepatch) and the
> > > klp_module_coming path (livepatch loaded before target).
> >
> > Summary:
> >
> > IMHO, this patch is perfectly fine as is if we accept that it will get
> > eventually obsoleted by my patchset (hopefully in a year or two).
> >
> > On the other hand, this patch would deserve some clean up,
> > (helper functions, don't die in the middle of the test) if
> > you planned to work on more tests. It would help to maintain
> > the tests.
> >
>
> Right, I think this was a good intro patch for Pablo and that the
> revised execution flow would be a great follow on series, if he is
> interested. How about that?
JFYI, I have committed the patch into livepatching.git,
branch for-7.1/module-function-test.
Best Regards,
Petr
^ permalink raw reply
* speaker-test-module: Re: [PATCH] selftests/livepatch: add test for module function patching
From: Petr Mladek @ 2026-03-30 13:48 UTC (permalink / raw)
To: Joe Lawrence
Cc: Pablo Hugen, live-patching, linux-kselftest, linux-kernel,
jpoimboe, jikos, mbenes, shuah
In-Reply-To: <acWZ3r2CoSDy_NLf@redhat.com>
On Thu 2026-03-26 16:41:02, Joe Lawrence wrote:
> On Thu, Mar 26, 2026 at 03:34:36PM +0100, Petr Mladek wrote:
> > On Fri 2026-03-20 17:11:17, Pablo Hugen wrote:
> > > From: Pablo Alessandro Santos Hugen <phugen@redhat.com>
> > In my RFC, I created a helper module which implemented a person
> > (speaker) which would come on the stage and welcome the audience.
> > I am not sure if it was a good idea. But it became a bit confusing
> > when everything (module name, sysfs interface, function name, message)
> > included the same strings like (livepatch, callback, shadow_var).
> >
> > Anyway, my tests produced messages like these:
> >
> > +% cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/welcome
> > +$MOD_TARGET: speaker_welcome: Hello, World!
> >
> > , see https://lore.kernel.org/all/20250115082431.5550-9-pmladek@suse.com/
> >
> >
> > There were even tests which blocked the transition. They tested shadow
> > variables which added an applause to the message. They did something like:
> >
> > <paste>
> > All four callbacks are used as follows:
> >
> > + pre_patch() allocates a shadow variable with a string and fills
> > it with "[]".
> > + post_patch() fills the string with "[APPLAUSE]".
> > + pre_unpatch() reverts the string back to "[]".
> > + post_unpatch() releases the shadow variable.
> >
> > The welcome message printed by the livepatched function allows us to
> > distinguish between the transition and the completed transition.
> > Specifically, the speaker's welcome message appears as:
> >
> > + Not patched system: "Hello, World!"
> > + Transition (unpatched task): "[] Hello, World!"
> > + Transition (patched task): "[] Ladies and gentlemen, ..."
> > + Patched system: "[APPLAUSE] Ladies and gentlemen, ..."
> > </paste>
> >
> > , see https://lore.kernel.org/all/20250115082431.5550-11-pmladek@suse.com/
> >
> > Sigh, I have done many changes in the tests for v1. But they still
> > need some love (and rebasing) for sending.
> >
>
> If these can be pulled out independently from the v1 patch, perhaps
> Pablo would want to hack on that in a follow up series?
Unfortunately, the selftest changes can't be pulled out easily.
The new test module and related livepatches are implemented using
the reworked livepatch API. :-/
I hope that I'll find time to work on it rather sooner than later.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command
From: Nicolas Schier @ 2026-03-30 18:49 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86, linux-kernel, live-patching, Peter Zijlstra, Joe Lawrence,
Song Liu, Catalin Marinas, Will Deacon, linux-arm-kernel,
Mark Rutland, Nathan Chancellor, Herbert Xu
In-Reply-To: <zdipyf26t2gos5dw2gjyzmeg2zm5a67xwr5ozubnhmhllrwgnm@ezdt54coe2bk>
On Wed, Mar 18, 2026 at 05:49:27PM -0700, Josh Poimboeuf wrote:
> On Wed, Mar 18, 2026 at 08:54:31PM +0100, Nicolas Schier wrote:
> > On Tue, Mar 17, 2026 at 03:51:07PM -0700, Josh Poimboeuf wrote:
> > > Split the objtool args into commands and options, such that if no
> > > commands have been enabled, objtool doesn't run.
> > >
> > > This is in preparation in enabling objtool and klp-build for arm64.
> > >
> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > > ---
> > > arch/x86/boot/startup/Makefile | 2 +-
> > > scripts/Makefile.build | 4 +--
> > > scripts/Makefile.lib | 46 ++++++++++++++++++----------------
> > > scripts/Makefile.vmlinux_o | 15 ++++-------
> > > 4 files changed, 33 insertions(+), 34 deletions(-)
> > >
> > [...]
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 3652b85be545..8a1bdfdb2fdb 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> > > is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
> > >
> > > ifdef CONFIG_OBJTOOL
> > > -$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
> > > +$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))
> >
> > Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if
> > $(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be
> > left-out:
> >
> > $(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))
>
> I believe that would break the !delay-objtool case. The logic needs to
> be something like:
>
> if (is-standard-object && objtool-cmds-y) {
> if (delay-objtool) {
> // for delay-objtool, only enable objtool for single-object modules
> $(is-single-obj-m)
> } else {
> // for !delay-objtool, always enable objtool
> y
> }
> }
>
> so maybe something like this?
>
> $(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y))
sorry for the delay! Yes, I overlooked the !delay-objtool. That line
looks good to me, thanks!
--
Nicolas
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Marcos Paulo de Souza @ 2026-03-31 18:54 UTC (permalink / raw)
To: Joe Lawrence
Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <abhjYtyveer4niGM@redhat.com>
On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> wrote:
> > Instead of checking if the architecture running the test was
> > powerpc,
> > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> >
> > No functional changes.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > index dd802783ea849..c01a586866304 100644
> > ---
> > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > +++
> > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > @@ -12,15 +12,14 @@
> > #include <linux/slab.h>
> > #include <linux/livepatch.h>
> >
> > -#if defined(__x86_64__)
> > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > +#define FN_PREFIX
> > +#elif defined(__x86_64__)
> > #define FN_PREFIX __x64_
> > #elif defined(__s390x__)
> > #define FN_PREFIX __s390x_
> > #elif defined(__aarch64__)
> > #define FN_PREFIX __arm64_
> > -#else
> > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > -#define FN_PREFIX
>
> The patch does maintain the previous behavior, but I'm wondering if
> the
> original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> correct:
>
> $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> !COMPAT
> depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
>
> Perhaps I just forgot what that additional piece of information that
> explains the comment (highly probable these days), and if so, might
> be
> nice to add to this commit since I don't see it in 6a71770442b5
> ("selftests: livepatch: Test livepatching a heavily called syscall").
Looking again at the code and at the symbols for SLE for ppc64le, I can
say that, even with ARCH_HAS_SYSCALL_WRAPPER being set, the syscall
names are not changed for ppc64le. Looking at
arch/powerpc/kernel/systbl.c:
#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
#define __SYSCALL(nr, entry) [nr] = entry,
#else
/*
* Coerce syscall handlers with arbitrary parameters to common type
* requires cast to void* to avoid -Wcast-function-type.
*/
#define __SYSCALL(nr, entry) [nr] = (void *) entry,
#endif
Differently from x86 and other already covered archs, the syscall name
remains the same on ppc. I will add a comment about it in the next
version, adding a case for ppc and an #else clause with an error, so
future architectures that support livepatching can add a patch here to
fix it for them.
>
> Thanks,
> --
> Joe
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Pablo Hugen @ 2026-03-31 20:52 UTC (permalink / raw)
To: Miroslav Benes, Joe Lawrence
Cc: Pablo Hugen, live-patching, linux-kselftest, linux-kernel,
jpoimboe, jikos, pmladek, shuah
In-Reply-To: <177442832293.70541.15179138173140080388.b4-reply@b4>
[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]
> We sort of test the same in test-callbacks.sh. Just using different
> means. I think I would not mind having this as well.
Ok. The original idea Joe suggested was to check the function output
of module targets directly. From what I can tell, test-callbacks.sh
covers the callbacks and test-ftrace.sh checks that ftrace
can still trace a function after it's been livepatched, neither
actually checks that a replacement function in a module target runs.
I can be wrong though, still getting familiar with the livepatch tests.
> I was *just* in the middle of replying to the patch when yours came in,
> so I'll just move over here. I had noticed the same thing re:
> test-callbacks.sh despite originally suggested writing this test to
> Pablo (and forgot about the callbacks test module). With that, I agree
> that it's a nice basic sanity check that's obvious about what it's
> testing.
Fair point. Altough biased I think it is nice to have this explict sanity check.
> A nit but is 'noinline' keyword needed here? proc_create_single() below
> takes a function pointer so hopefully test_klp_mod_target_show() stays
> even without it?
> No strong preference either way. I won't fault a livepatch developer
> for being paranoid w/respect to the compiler :D
Yeah I think you're right, not strictly needed, just wanted to be sure.
After some experience rebasing kpatch integration tests I've been bitten
enough times to be paranoid about that :)
But since I will work on a follow-up for the other suggestions, I think
I'll drop it there.
Anyways, thanks for the reviews!
Pablo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] selftests/livepatch: add test for module function patching
From: Pablo Hugen @ 2026-03-31 21:10 UTC (permalink / raw)
To: Miroslav Benes, Joe Lawrence
Cc: Petr Mladek, Pablo Hugen, live-patching, linux-kselftest,
linux-kernel, jpoimboe, jikos, shuah
In-Reply-To: <alpine.LSU.2.21.2603271143310.31210@pobox.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 908 bytes --]
> > Summary:
> >
> > IMHO, this patch is perfectly fine as is if we accept that it will get
> > eventually obsoleted by my patchset (hopefully in a year or two).
> >
> > On the other hand, this patch would deserve some clean up,
> > (helper functions, don't die in the middle of the test) if
> > you planned to work on more tests. It would help to maintain
> > the tests.
> Right, I think this was a good intro patch for Pablo and that the
> revised execution flow would be a great follow on series, if he is
> interested. How about that?
Sure, will take a stab at revising the cleanup flow. And thanks for the ideas.
> > This code is repeated several times. It might be worth creating a
> > helper function in tools/testing/selftests/livepatch/functions.sh.
Makes sense. Will include in the follow-up.
Thanks for the reviews everyone, and thanks Petr for picking it up.
Pablo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/8] selftests: livepatch: test-syscall: Check for ARCH_HAS_SYSCALL_WRAPPER
From: Miroslav Benes @ 2026-04-01 7:41 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Joe Lawrence, Josh Poimboeuf, Jiri Kosina, Petr Mladek,
Shuah Khan, live-patching, linux-kselftest, linux-kernel
In-Reply-To: <a23f00ad4a454cbb3379cdf512e8c61ce7499194.camel@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]
On Tue, 31 Mar 2026, Marcos Paulo de Souza wrote:
> On Mon, 2026-03-16 at 16:12 -0400, Joe Lawrence wrote:
> > On Fri, Mar 13, 2026 at 05:58:32PM -0300, Marcos Paulo de Souza
> > wrote:
> > > Instead of checking if the architecture running the test was
> > > powerpc,
> > > check if CONF_ARCH_HAS_SYSCALL_WRAPPER is defined or not.
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > > ---
> > > tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > index dd802783ea849..c01a586866304 100644
> > > ---
> > > a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > +++
> > > b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
> > > @@ -12,15 +12,14 @@
> > > #include <linux/slab.h>
> > > #include <linux/livepatch.h>
> > >
> > > -#if defined(__x86_64__)
> > > +#if !defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER)
> > > +#define FN_PREFIX
> > > +#elif defined(__x86_64__)
> > > #define FN_PREFIX __x64_
> > > #elif defined(__s390x__)
> > > #define FN_PREFIX __s390x_
> > > #elif defined(__aarch64__)
> > > #define FN_PREFIX __arm64_
> > > -#else
> > > -/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
> > > -#define FN_PREFIX
> >
> > The patch does maintain the previous behavior, but I'm wondering if
> > the
> > original assertion about ARCH_HAS_SYSCALL_WRAPPER on Power was
> > correct:
> >
> > $ grep ARCH_HAS_SYSCALL_WRAPPER arch/powerpc/Kconfig
> > select ARCH_HAS_SYSCALL_WRAPPER if !SPU_BASE &&
> > !COMPAT
> > depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> >
> > Perhaps I just forgot what that additional piece of information that
> > explains the comment (highly probable these days), and if so, might
> > be
> > nice to add to this commit since I don't see it in 6a71770442b5
> > ("selftests: livepatch: Test livepatching a heavily called syscall").
>
> Looking again at the code and at the symbols for SLE for ppc64le, I can
> say that, even with ARCH_HAS_SYSCALL_WRAPPER being set, the syscall
> names are not changed for ppc64le. Looking at
> arch/powerpc/kernel/systbl.c:
>
> #ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> #define __SYSCALL(nr, entry) [nr] = entry,
> #else
> /*
> * Coerce syscall handlers with arbitrary parameters to common type
> * requires cast to void* to avoid -Wcast-function-type.
> */
> #define __SYSCALL(nr, entry) [nr] = (void *) entry,
> #endif
I think this is not the complete picture though. The definition of the
syscall table did not change but the actual wrappers (or syscall functions
naming before) did change a couple of times even on powerpc if I am
reading the git history right. ARCH_HAS_SYSCALL_WRAPPER also changed how
syscall parameters are consumed (from registers to stack frame (pt_regs)).
It is not particularly relevant for getpid() which is SYSCALL_DEFINE0()
but I wanted to point that out.
Miroslav
^ permalink raw reply
* [RFC PATCH 0/4] trace, livepatch: Allow kprobe return overriding for livepatched functions
From: Yafang Shao @ 2026-04-02 9:26 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song
Cc: live-patching, linux-kernel, linux-trace-kernel, bpf, Yafang Shao
Livepatching allows for rapid experimentation with new kernel features
without interrupting production workloads. However, static livepatches lack
the flexibility required to tune features based on task-specific attributes,
such as cgroup membership, which is critical in multi-tenant k8s
environments. Furthermore, hardcoding logic into a livepatch prevents
dynamic adjustments based on the runtime environment.
To address this, we propose a hybrid approach using BPF. Our production use
case involves:
1. Deploying a Livepatch function to serve as a stable BPF hook.
2. Utilizing bpf_override_return() to dynamically modify the return value
of that hook based on the current task's context.
A significant challenge arises when atomic-replace is enabled. In this
mode, deploying a new livepatch changes the target function's address,
forcing a re-attachment of the BPF program. This re-attachment latency is
unacceptable in critical paths, such as those handling networking policies.
To solve this, we introduce a hybrid livepatch mode that allows specific
patches to remain non-replaceable, ensuring the function address remains
stable and the BPF program stays attached.
Furthermore, this mechanism provides a lower-maintenance alternative to
out-of-tree BPF hooks. Given the complexities of upstreaming custom BPF
hooks (e.g., [0], [1]), this hybrid mode allows for the maintenance of
stable, minimal hook points via livepatching with significantly reduced
maintenance burden.
Link: https://lwn.net/Articles/1054030/ [0]
Link: https://lwn.net/Articles/1043548/ [1]
Yafang Shao (4):
trace: Simplify kprobe overridable function check
trace: Allow kprobes to override livepatched functions
livepatch: Add "replaceable" attribute to klp_patch
livepatch: Implement livepatch hybrid mode
include/linux/livepatch.h | 2 ++
kernel/livepatch/core.c | 50 +++++++++++++++++++++++++++++++
kernel/trace/Kconfig | 14 +++++++++
kernel/trace/bpf_trace.c | 14 ++++++---
kernel/trace/trace_kprobe.c | 49 ++++++++++++------------------
kernel/trace/trace_probe.h | 59 +++++++++++++++++++++++++++----------
6 files changed, 139 insertions(+), 49 deletions(-)
--
2.47.3
^ permalink raw reply
* [RFC PATCH 1/4] trace: Simplify kprobe overridable function check
From: Yafang Shao @ 2026-04-02 9:26 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song
Cc: live-patching, linux-kernel, linux-trace-kernel, bpf, Yafang Shao
In-Reply-To: <20260402092607.96430-1-laoar.shao@gmail.com>
Simplify the logic for checking overridable kprobe functions by removing
redundant code.
No functional change.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/trace/bpf_trace.c | 13 ++++++---
kernel/trace/trace_kprobe.c | 40 +++++----------------------
kernel/trace/trace_probe.h | 54 ++++++++++++++++++++++++++-----------
3 files changed, 54 insertions(+), 53 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0b040a417442..c901ace836cb 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1929,10 +1929,15 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
* Kprobe override only works if they are on the function entry,
* and only if they are on the opt-in list.
*/
- if (prog->kprobe_override &&
- (!trace_kprobe_on_func_entry(event->tp_event) ||
- !trace_kprobe_error_injectable(event->tp_event)))
- return -EINVAL;
+ if (prog->kprobe_override) {
+ struct trace_kprobe *tp = trace_kprobe_primary_from_call(event->tp_event);
+
+ if (!tp)
+ return -EINVAL;
+ if (!trace_kprobe_on_func_entry(tp) ||
+ !trace_kprobe_error_injectable(tp))
+ return -EINVAL;
+ }
mutex_lock(&bpf_event_mutex);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a5dbb72528e0..768702674a5c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -53,17 +53,6 @@ static struct dyn_event_operations trace_kprobe_ops = {
.match = trace_kprobe_match,
};
-/*
- * Kprobe event core functions
- */
-struct trace_kprobe {
- struct dyn_event devent;
- struct kretprobe rp; /* Use rp.kp for kprobe use */
- unsigned long __percpu *nhit;
- const char *symbol; /* symbol name */
- struct trace_probe tp;
-};
-
static bool is_trace_kprobe(struct dyn_event *ev)
{
return ev->ops == &trace_kprobe_ops;
@@ -212,33 +201,16 @@ unsigned long trace_kprobe_address(struct trace_kprobe *tk)
return addr;
}
-static nokprobe_inline struct trace_kprobe *
-trace_kprobe_primary_from_call(struct trace_event_call *call)
-{
- struct trace_probe *tp;
-
- tp = trace_probe_primary_from_call(call);
- if (WARN_ON_ONCE(!tp))
- return NULL;
-
- return container_of(tp, struct trace_kprobe, tp);
-}
-
-bool trace_kprobe_on_func_entry(struct trace_event_call *call)
+bool trace_kprobe_on_func_entry(struct trace_kprobe *tp)
{
- struct trace_kprobe *tk = trace_kprobe_primary_from_call(call);
-
- return tk ? (kprobe_on_func_entry(tk->rp.kp.addr,
- tk->rp.kp.addr ? NULL : tk->rp.kp.symbol_name,
- tk->rp.kp.addr ? 0 : tk->rp.kp.offset) == 0) : false;
+ return !kprobe_on_func_entry(tp->rp.kp.addr,
+ tp->rp.kp.addr ? NULL : tp->rp.kp.symbol_name,
+ tp->rp.kp.addr ? 0 : tp->rp.kp.offset);
}
-bool trace_kprobe_error_injectable(struct trace_event_call *call)
+bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
{
- struct trace_kprobe *tk = trace_kprobe_primary_from_call(call);
-
- return tk ? within_error_injection_list(trace_kprobe_address(tk)) :
- false;
+ return within_error_injection_list(trace_kprobe_address(tp));
}
static int register_kprobe_event(struct trace_kprobe *tk);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 9fc56c937130..958eb78a9068 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -30,6 +30,7 @@
#include "trace.h"
#include "trace_output.h"
+#include "trace_dynevent.h"
#define MAX_TRACE_ARGS 128
#define MAX_ARGSTR_LEN 63
@@ -210,21 +211,6 @@ DECLARE_BASIC_PRINT_TYPE_FUNC(symbol);
#define ASSIGN_FETCH_TYPE_END {}
#define MAX_ARRAY_LEN 64
-#ifdef CONFIG_KPROBE_EVENTS
-bool trace_kprobe_on_func_entry(struct trace_event_call *call);
-bool trace_kprobe_error_injectable(struct trace_event_call *call);
-#else
-static inline bool trace_kprobe_on_func_entry(struct trace_event_call *call)
-{
- return false;
-}
-
-static inline bool trace_kprobe_error_injectable(struct trace_event_call *call)
-{
- return false;
-}
-#endif /* CONFIG_KPROBE_EVENTS */
-
struct probe_arg {
struct fetch_insn *code;
bool dynamic;/* Dynamic array (string) is used */
@@ -271,6 +257,32 @@ struct event_file_link {
struct list_head list;
};
+/*
+ * Kprobe event core functions
+ */
+struct trace_kprobe {
+ struct dyn_event devent;
+ struct kretprobe rp; /* Use rp.kp for kprobe use */
+ unsigned long __percpu *nhit;
+ const char *symbol; /* symbol name */
+ struct trace_probe tp;
+};
+
+#ifdef CONFIG_KPROBE_EVENTS
+bool trace_kprobe_on_func_entry(struct trace_kprobe *tp);
+bool trace_kprobe_error_injectable(struct trace_kprobe *tp);
+#else
+static inline bool trace_kprobe_on_func_entry(struct trace_kprobe *tp)
+{
+ return false;
+}
+
+static inline bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
+{
+ return false;
+}
+#endif /* CONFIG_KPROBE_EVENTS */
+
static inline unsigned int trace_probe_load_flag(struct trace_probe *tp)
{
return smp_load_acquire(&tp->event->flags);
@@ -329,6 +341,18 @@ trace_probe_primary_from_call(struct trace_event_call *call)
return list_first_entry_or_null(&tpe->probes, struct trace_probe, list);
}
+static nokprobe_inline struct trace_kprobe *
+trace_kprobe_primary_from_call(struct trace_event_call *call)
+{
+ struct trace_probe *tp;
+
+ tp = trace_probe_primary_from_call(call);
+ if (WARN_ON_ONCE(!tp))
+ return NULL;
+
+ return container_of(tp, struct trace_kprobe, tp);
+}
+
static inline struct list_head *trace_probe_probe_list(struct trace_probe *tp)
{
return &tp->event->probes;
--
2.47.3
^ permalink raw reply related
* [RFC PATCH 2/4] trace: Allow kprobes to override livepatched functions
From: Yafang Shao @ 2026-04-02 9:26 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song
Cc: live-patching, linux-kernel, linux-trace-kernel, bpf, Yafang Shao
In-Reply-To: <20260402092607.96430-1-laoar.shao@gmail.com>
Introduce the ability for kprobes to override the return values of
functions that have been livepatched. This functionality is guarded by the
CONFIG_KPROBE_OVERRIDE_KLP_FUNC configuration option.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/trace/Kconfig | 14 ++++++++++++++
kernel/trace/bpf_trace.c | 3 ++-
kernel/trace/trace_kprobe.c | 17 +++++++++++++++++
kernel/trace/trace_probe.h | 5 +++++
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 49de13cae428..db712c8cb745 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -1279,6 +1279,20 @@ config HIST_TRIGGERS_DEBUG
If unsure, say N.
+config KPROBE_OVERRIDE_KLP_FUNC
+ bool "Allow kprobes to override livepatched functions"
+ depends on KPROBES && LIVEPATCH
+ help
+ This option allows BPF programs to use kprobes to override functions
+ that have already been patched by Livepatch (KLP).
+
+ Enabling this provides a mechanism to dynamically control execution
+ flow without requiring a reboot or a new livepatch module. It
+ effectively combines the persistence of livepatching with the
+ programmability of BPF.
+
+ If unsure, say N.
+
source "kernel/trace/rv/Kconfig"
endif # FTRACE
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c901ace836cb..08ae2b1a912c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1935,7 +1935,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
if (!tp)
return -EINVAL;
if (!trace_kprobe_on_func_entry(tp) ||
- !trace_kprobe_error_injectable(tp))
+ (!trace_kprobe_error_injectable(tp) &&
+ !trace_kprobe_klp_func_overridable(tp)))
return -EINVAL;
}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 768702674a5c..6f05451fbc76 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -213,6 +213,23 @@ bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
return within_error_injection_list(trace_kprobe_address(tp));
}
+bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp)
+{
+ bool overridable = false;
+#ifdef CONFIG_KPROBE_OVERRIDE_KLP_FUNC
+ struct module *mod;
+ unsigned long addr;
+
+ addr = trace_kprobe_address(tp);
+ rcu_read_lock();
+ mod = __module_address(addr);
+ if (mod && mod->klp)
+ overridable = true;
+ rcu_read_unlock();
+#endif
+ return overridable;
+}
+
static int register_kprobe_event(struct trace_kprobe *tk);
static int unregister_kprobe_event(struct trace_kprobe *tk);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 958eb78a9068..84bd2617db7c 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -271,6 +271,7 @@ struct trace_kprobe {
#ifdef CONFIG_KPROBE_EVENTS
bool trace_kprobe_on_func_entry(struct trace_kprobe *tp);
bool trace_kprobe_error_injectable(struct trace_kprobe *tp);
+bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp);
#else
static inline bool trace_kprobe_on_func_entry(struct trace_kprobe *tp)
{
@@ -281,6 +282,10 @@ static inline bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
{
return false;
}
+static inline bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp)
+{
+ return false;
+}
#endif /* CONFIG_KPROBE_EVENTS */
static inline unsigned int trace_probe_load_flag(struct trace_probe *tp)
--
2.47.3
^ permalink raw reply related
* [RFC PATCH 3/4] livepatch: Add "replaceable" attribute to klp_patch
From: Yafang Shao @ 2026-04-02 9:26 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song
Cc: live-patching, linux-kernel, linux-trace-kernel, bpf, Yafang Shao
In-Reply-To: <20260402092607.96430-1-laoar.shao@gmail.com>
Add a new replaceable attribute to allow the coexistence of both
atomic-replace and non-atomic-replace livepatches. If replaceable is set to
0, the livepatch will not be replaced by a subsequent atomic-replace
operation.
This is a preparatory patch for following changes.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/linux/livepatch.h | 2 ++
kernel/livepatch/core.c | 44 +++++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index ba9e3988c07c..d88a6966e5f2 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -124,6 +124,7 @@ struct klp_state {
* @objs: object entries for kernel objects to be patched
* @states: system states that can get modified
* @replace: replace all actively used patches
+ * @replaceable: whether this patch can be replaced or not
* @list: list node for global list of actively used patches
* @kobj: kobject for sysfs resources
* @obj_list: dynamic list of the object entries
@@ -138,6 +139,7 @@ struct klp_patch {
struct klp_object *objs;
struct klp_state *states;
bool replace;
+ bool replaceable;
/* internal */
struct list_head list;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 28d15ba58a26..04f9e84f114f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -351,6 +351,7 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
* /sys/kernel/livepatch/<patch>/transition
* /sys/kernel/livepatch/<patch>/force
* /sys/kernel/livepatch/<patch>/replace
+ * /sys/kernel/livepatch/<patch>/replaceable
* /sys/kernel/livepatch/<patch>/stack_order
* /sys/kernel/livepatch/<patch>/<object>
* /sys/kernel/livepatch/<patch>/<object>/patched
@@ -478,17 +479,60 @@ static ssize_t stack_order_show(struct kobject *kobj,
return sysfs_emit(buf, "%d\n", stack_order);
}
+static ssize_t replaceable_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct klp_patch *patch;
+ bool replaceable;
+ int ret;
+
+ ret = kstrtobool(buf, &replaceable);
+ if (ret)
+ return ret;
+
+ patch = container_of(kobj, struct klp_patch, kobj);
+
+ mutex_lock(&klp_mutex);
+
+ if (patch->replaceable == replaceable)
+ goto out;
+
+ if (patch == klp_transition_patch) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
+ patch->replaceable = replaceable;
+
+out:
+ mutex_unlock(&klp_mutex);
+
+ if (ret)
+ return ret;
+ return count;
+}
+static ssize_t replaceable_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct klp_patch *patch;
+
+ patch = container_of(kobj, struct klp_patch, kobj);
+ return sysfs_emit(buf, "%d\n", patch->replaceable);
+}
+
static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
static struct kobj_attribute replace_kobj_attr = __ATTR_RO(replace);
static struct kobj_attribute stack_order_kobj_attr = __ATTR_RO(stack_order);
+static struct kobj_attribute replaceable_kobj_attr = __ATTR_RW(replaceable);
static struct attribute *klp_patch_attrs[] = {
&enabled_kobj_attr.attr,
&transition_kobj_attr.attr,
&force_kobj_attr.attr,
&replace_kobj_attr.attr,
&stack_order_kobj_attr.attr,
+ &replaceable_kobj_attr.attr,
NULL
};
ATTRIBUTE_GROUPS(klp_patch);
--
2.47.3
^ permalink raw reply related
* [RFC PATCH 4/4] livepatch: Implement livepatch hybrid mode
From: Yafang Shao @ 2026-04-02 9:26 UTC (permalink / raw)
To: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song
Cc: live-patching, linux-kernel, linux-trace-kernel, bpf, Yafang Shao
In-Reply-To: <20260402092607.96430-1-laoar.shao@gmail.com>
Livepatching allows for rapid experimentation with new kernel features
without interrupting production workloads. However, static livepatches lack
the flexibility required to tune features based on task-specific attributes,
such as cgroup membership, which is critical in multi-tenant k8s
environments. Furthermore, hardcoding logic into a livepatch prevents
dynamic adjustments based on the runtime environment.
To address this, we propose a hybrid approach using BPF. Our production use
case involves:
1. Deploying a Livepatch function to serve as a stable BPF hook.
2. Utilizing bpf_override_return() to dynamically modify the return value
of that hook based on the current task's context.
A significant challenge arises when atomic-replace is enabled. In this
mode, deploying a new livepatch changes the target function's address,
forcing a re-attachment of the BPF program. This re-attachment latency is
unacceptable in critical paths, such as those handling networking policies.
To solve this, we introduce a hybrid livepatch mode that allows specific
patches to remain non-replaceable, ensuring the function address remains
stable and the BPF program stays attached.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/livepatch/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 04f9e84f114f..5a44154131c8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -665,6 +665,8 @@ static int klp_add_nops(struct klp_patch *patch)
klp_for_each_object(old_patch, old_obj) {
int err;
+ if (!old_patch->replaceable)
+ continue;
err = klp_add_object_nops(patch, old_obj);
if (err)
return err;
@@ -837,6 +839,8 @@ void klp_free_replaced_patches_async(struct klp_patch *new_patch)
klp_for_each_patch_safe(old_patch, tmp_patch) {
if (old_patch == new_patch)
return;
+ if (!old_patch->replaceable)
+ continue;
klp_free_patch_async(old_patch);
}
}
@@ -1239,6 +1243,8 @@ void klp_unpatch_replaced_patches(struct klp_patch *new_patch)
if (old_patch == new_patch)
return;
+ if (!old_patch->replaceable)
+ continue;
old_patch->enabled = false;
klp_unpatch_objects(old_patch);
}
--
2.47.3
^ permalink raw reply related
* Re: [RFC PATCH 2/4] trace: Allow kprobes to override livepatched functions
From: Menglong Dong @ 2026-04-02 12:48 UTC (permalink / raw)
To: Yafang Shao
Cc: jpoimboe, jikos, mbenes, pmladek, joe.lawrence, rostedt, mhiramat,
mathieu.desnoyers, kpsingh, mattbobrowski, song, jolsa, ast,
daniel, andrii, martin.lau, eddyz87, memxor, yonghong.song,
Yafang Shao, live-patching, linux-kernel, linux-trace-kernel, bpf
In-Reply-To: <20260402092607.96430-3-laoar.shao@gmail.com>
On 2026/4/2 17:26, Yafang Shao wrote:
> Introduce the ability for kprobes to override the return values of
> functions that have been livepatched. This functionality is guarded by the
> CONFIG_KPROBE_OVERRIDE_KLP_FUNC configuration option.
Hi, Yafang. This is a interesting idea.
For now, the bpf_override_return() can only be used on the kernel
functions that allow error injection to prevent the BPF program from
crash the kernel. If we use it on the kernel functions that patched
by the KLP, we can crash the kernel easily by return a invalid value
with bpf_override_return(), right? (Of course, we can crash the kernel
easily with KLP too ;)
I haven't figure out the use case yet. Can KLP be used together with
the BPF program that use bpf_override_return()? The KLP will modify
the RIP on the stack, and the bpf_override_return() will modify it too.
AFAIK, there can't be two ftrace_ops that both have the
FTRACE_OPS_FL_IPMODIFY flag. Did I miss something?
It will be helpful for me to understand the use case if a selftests is
offered :)
BTW, if we allow the usage of bpf_override_return() on the KLP patched
function, we should allow the usage of BPF_MODIFY_RETURN on this
case too, right?
Thanks!
Menglong Dong
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> kernel/trace/Kconfig | 14 ++++++++++++++
> kernel/trace/bpf_trace.c | 3 ++-
> kernel/trace/trace_kprobe.c | 17 +++++++++++++++++
> kernel/trace/trace_probe.h | 5 +++++
> 4 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 49de13cae428..db712c8cb745 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -1279,6 +1279,20 @@ config HIST_TRIGGERS_DEBUG
>
> If unsure, say N.
>
> +config KPROBE_OVERRIDE_KLP_FUNC
> + bool "Allow kprobes to override livepatched functions"
> + depends on KPROBES && LIVEPATCH
> + help
> + This option allows BPF programs to use kprobes to override functions
> + that have already been patched by Livepatch (KLP).
> +
> + Enabling this provides a mechanism to dynamically control execution
> + flow without requiring a reboot or a new livepatch module. It
> + effectively combines the persistence of livepatching with the
> + programmability of BPF.
> +
> + If unsure, say N.
> +
> source "kernel/trace/rv/Kconfig"
>
> endif # FTRACE
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index c901ace836cb..08ae2b1a912c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1935,7 +1935,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
> if (!tp)
> return -EINVAL;
> if (!trace_kprobe_on_func_entry(tp) ||
> - !trace_kprobe_error_injectable(tp))
> + (!trace_kprobe_error_injectable(tp) &&
> + !trace_kprobe_klp_func_overridable(tp)))
> return -EINVAL;
> }
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 768702674a5c..6f05451fbc76 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -213,6 +213,23 @@ bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
> return within_error_injection_list(trace_kprobe_address(tp));
> }
>
> +bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp)
> +{
> + bool overridable = false;
> +#ifdef CONFIG_KPROBE_OVERRIDE_KLP_FUNC
> + struct module *mod;
> + unsigned long addr;
> +
> + addr = trace_kprobe_address(tp);
> + rcu_read_lock();
> + mod = __module_address(addr);
> + if (mod && mod->klp)
> + overridable = true;
> + rcu_read_unlock();
> +#endif
> + return overridable;
> +}
> +
> static int register_kprobe_event(struct trace_kprobe *tk);
> static int unregister_kprobe_event(struct trace_kprobe *tk);
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 958eb78a9068..84bd2617db7c 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -271,6 +271,7 @@ struct trace_kprobe {
> #ifdef CONFIG_KPROBE_EVENTS
> bool trace_kprobe_on_func_entry(struct trace_kprobe *tp);
> bool trace_kprobe_error_injectable(struct trace_kprobe *tp);
> +bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp);
> #else
> static inline bool trace_kprobe_on_func_entry(struct trace_kprobe *tp)
> {
> @@ -281,6 +282,10 @@ static inline bool trace_kprobe_error_injectable(struct trace_kprobe *tp)
> {
> return false;
> }
> +static inline bool trace_kprobe_klp_func_overridable(struct trace_kprobe *tp)
> +{
> + return false;
> +}
> #endif /* CONFIG_KPROBE_EVENTS */
>
> static inline unsigned int trace_probe_load_flag(struct trace_probe *tp)
>
^ 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