selftests/livepatch: add test for module function patching

PATCH In review Pablo Hugen 4 participants 11 messages Mar 20 – Mar 31

Subsystems (2)

Kselftest Live patching

lore

Share & tools

lore

Pablo Hugen

author

2026-03-20 20:14 UTC #0 lore raw

PATCH 1/1 selftests/livepatch: add test for module function patching

From: Pablo Alessandro Santos Hugen <[email protected]>

Add a target module and livepatch pair that verify module function patching via a proc entry. Two test cases cover both the klp_enable_patch path (target loaded before livepatch) and the klp_module_coming path (livepatch loaded before target).

Signed-off-by Pablo Alessandro Santos Hugen <[email protected]>

---
 .../selftests/livepatch/test-livepatch.sh     | 100 ++++++++++++++++++
 .../selftests/livepatch/test_modules/Makefile |   2 +
 .../test_modules/test_klp_mod_patch.c         |  53 ++++++++++
 .../test_modules/test_klp_mod_target.c        |  39 +++++++
 4 files changed, 194 insertions(+)
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c

tools/testing/selftests/livepatch/test-livepatch.sh +100 -0

index 6673023d2b66..c44c5341a2f1 100755
@@ -8,6 +8,8 @@ MOD_LIVEPATCH1=test_klp_livepatch
 MOD_LIVEPATCH2=test_klp_syscall
 MOD_LIVEPATCH3=test_klp_callbacks_demo
 MOD_REPLACE=test_klp_atomic_replace
+MOD_TARGET=test_klp_mod_target
+MOD_TARGET_PATCH=test_klp_mod_patch
 
 setup_config

@@ -196,4 +198,102 @@ livepatch: '$MOD_REPLACE': unpatching complete
 % rmmod $MOD_REPLACE"
 
 
+# - load a target module that provides /proc/test_klp_mod_target with
+#   original output
+# - load a livepatch that patches the target module's show function
+# - verify the proc entry returns livepatched output
+# - disable and unload the livepatch
+# - verify the proc entry returns original output again
+# - unload the target module
+
+start_test "module function patching"
+
+load_mod $MOD_TARGET
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+load_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+disable_lp $MOD_TARGET_PATCH
+unload_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+unload_mod $MOD_TARGET
+
+check_result "% insmod test_modules/$MOD_TARGET.ko
+$MOD_TARGET: test_klp_mod_target_init
+% insmod test_modules/$MOD_TARGET_PATCH.ko
+livepatch: enabling patch '$MOD_TARGET_PATCH'
+livepatch: '$MOD_TARGET_PATCH': initializing patching transition
+livepatch: '$MOD_TARGET_PATCH': starting patching transition
+livepatch: '$MOD_TARGET_PATCH': completing patching transition
+livepatch: '$MOD_TARGET_PATCH': patching complete
+% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
+livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
+livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': unpatching complete
+% rmmod $MOD_TARGET_PATCH
+% rmmod $MOD_TARGET
+$MOD_TARGET: test_klp_mod_target_exit"
+
+
+# - load a livepatch that targets a not-yet-loaded module
+# - load the target module: klp_module_coming patches it immediately
+# - verify the proc entry returns livepatched output
+# - disable and unload the livepatch
+# - verify the proc entry returns original output again
+# - unload the target module
+
+start_test "module function patching (livepatch first)"
+
+load_lp $MOD_TARGET_PATCH
+load_mod $MOD_TARGET
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+disable_lp $MOD_TARGET_PATCH
+unload_lp $MOD_TARGET_PATCH
+
+if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+unload_mod $MOD_TARGET
+
+check_result "% insmod test_modules/$MOD_TARGET_PATCH.ko
+livepatch: enabling patch '$MOD_TARGET_PATCH'
+livepatch: '$MOD_TARGET_PATCH': initializing patching transition
+livepatch: '$MOD_TARGET_PATCH': starting patching transition
+livepatch: '$MOD_TARGET_PATCH': completing patching transition
+livepatch: '$MOD_TARGET_PATCH': patching complete
+% insmod test_modules/$MOD_TARGET.ko
+livepatch: applying patch '$MOD_TARGET_PATCH' to loading module '$MOD_TARGET'
+$MOD_TARGET: test_klp_mod_target_init
+% echo 0 > $SYSFS_KLP_DIR/$MOD_TARGET_PATCH/enabled
+livepatch: '$MOD_TARGET_PATCH': initializing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': starting unpatching transition
+livepatch: '$MOD_TARGET_PATCH': completing unpatching transition
+livepatch: '$MOD_TARGET_PATCH': unpatching complete
+% rmmod $MOD_TARGET_PATCH
+% rmmod $MOD_TARGET
+$MOD_TARGET: test_klp_mod_target_exit"
+
+
 exit 0

tools/testing/selftests/livepatch/test_modules/Makefile +2 -0

index 939230e571f5..a13d398585dc 100644
@@ -8,6 +8,8 @@ obj-m += test_klp_atomic_replace.o \
 	test_klp_callbacks_mod.o \
 	test_klp_kprobe.o \
 	test_klp_livepatch.o \
+	test_klp_mod_patch.o \
+	test_klp_mod_target.o \
 	test_klp_shadow_vars.o \
 	test_klp_state.o \
 	test_klp_state2.o \

tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c new file +53 -0

53 lines not on this page · raw message

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +39 -0

new file mode 100644
index 000000000000..9643984d2402
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Pablo Hugen <[email protected]>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+
+static struct proc_dir_entry *pde;
+
+static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
+	return 0;
+}
+
+static int test_klp_mod_target_init(void)
+{
+	pr_info("%s\n", __func__);
+	pde = proc_create_single("test_klp_mod_target", 0, NULL,
+				 test_klp_mod_target_show);
+	if (!pde)
+		return -ENOMEM;
+	return 0;
+}
+
+static void test_klp_mod_target_exit(void)
+{
+	pr_info("%s\n", __func__);
+	proc_remove(pde);
+}
+
+module_init(test_klp_mod_target_init);
+module_exit(test_klp_mod_target_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Hugen <[email protected]>");
+MODULE_DESCRIPTION("Livepatch test: target module with proc entry");

Signature · 1 line

2.53.0

Miroslav Benes

maintainer

2026-03-24 14:22 UTC #1 lore raw

Quoted 4 lines · Pablo Hugen

Add a target module and livepatch pair that verify module function patching via a proc entry. Two test cases cover both the klp_enable_patch path (target loaded before livepatch) and the klp_module_coming path (livepatch loaded before target).

We sort of test the same in test-callbacks.sh. Just using different means. I think I would not mind having this as well.

Petr, Joe, what do you think?

Quoted 9 lines

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +0 -0
new file mode 100644
index 000000000000..9643984d2402
@@ -0,0 +1,39 @@
[ ... skip 11 lines ... ]

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c line 17

Quoted 6 lines

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +6 -0

+
+static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
+	return 0;
+}

A nit but is 'noinline' keyword needed here? proc_create_single() below takes a function pointer so hopefully test_klp_mod_target_show() stays even without it?

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c line 23

Quoted 6 lines

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +6 -0

+
+static int test_klp_mod_target_init(void)
+{
+	pr_info("%s\n", __func__);
+	pde = proc_create_single("test_klp_mod_target", 0, NULL,
+				 test_klp_mod_target_show);

...here.

Otherwise it looks good to me.

Signature · 1 line

Miroslav

Joe Lawrence

maintainer Acked-by

2026-03-24 14:45 UTC #2 lore raw

Quoted 11 lines · Miroslav Benes

Quoted 4 lines · Pablo Hugen
Add a target module and livepatch pair that verify module function patching via a proc entry. Two test cases cover both the klp_enable_patch path (target loaded before livepatch) and the klp_module_coming path (livepatch loaded before target).
We sort of test the same in test-callbacks.sh. Just using different means. I think I would not mind having this as well.
Petr, Joe, what do you think?

I was *just* in the middle of replying to the patch when yours came in, so I'll just move over here. I had noticed the same thing re:
test-callbacks.sh despite originally suggested writing this test to Pablo (and forgot about the callbacks test module). With that, I agree that it's a nice basic sanity check that's obvious about what it's testing.

Quoted 11 lines

Quoted 11 lines
tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +0 -0
new file mode 100644
index 000000000000..9643984d2402
@@ -0,0 +1,39 @@
[ ... skip 11 lines ... ]
tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +2 -0
+
+static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)

Quoted 9 lines

tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c line 17
Quoted 4 lines
tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c new file +4 -0
+{
+	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
+	return 0;
+}
A nit but is 'noinline' keyword needed here? proc_create_single() below takes a function pointer so hopefully test_klp_mod_target_show() stays even without it?

No strong preference either way. I won't fault a livepatch developer for being paranoid w/respect to the compiler :D

Acked-by Joe Lawrence <[email protected]>

--
Joe

Miroslav Benes

maintainer Acked-by

2026-03-25 08:45 UTC #3 lore raw

Quoted 6 lines

Quoted 3 lines
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 <[email protected]>

Pablo Hugen

author

2026-03-31 20:52 UTC #4 lore raw

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

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

Petr Mladek

maintainer

2026-03-26 14:34 UTC #5 lore raw

In reply to Pablo Hugen

Quoted 6 lines · Pablo Hugen

From: Pablo Alessandro Santos Hugen <[email protected]>
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/[email protected]/

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.

Quoted 21 lines

tools/testing/selftests/livepatch/test-livepatch.sh +8 -0

@@ -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

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 216

Quoted 10 lines

tools/testing/selftests/livepatch/test-livepatch.sh +10 -0

+# - 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.

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 223

Quoted 6 lines

tools/testing/selftests/livepatch/test-livepatch.sh +6 -0

+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.

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 237

Quoted 13 lines

tools/testing/selftests/livepatch/test-livepatch.sh +13 -0

+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/[email protected]/

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/[email protected]/

Sigh, I have done many changes in the tests for v1. But they still need some love (and rebasing) for sending.

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 250

Quoted 13 lines

tools/testing/selftests/livepatch/test-livepatch.sh +13 -0

+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

Joe Lawrence

maintainer

2026-03-26 20:41 UTC #6 lore raw

Quoted 10 lines · Petr Mladek

Quoted 6 lines · Pablo Hugen
From: Pablo Alessandro Santos Hugen <[email protected]>
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.

Quoted 8 lines

Second, I am a bit biased because I am working on a patchset which would obsolete this patch, see
https://lore.kernel.org/all/[email protected]/
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.

Quoted 38 lines

Your test might be perfectly fine in the meantime. Just see few notes below.
Quoted 21 lines
tools/testing/selftests/livepatch/test-livepatch.sh +8 -0
@@ -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
tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 216
Quoted 10 lines
tools/testing/selftests/livepatch/test-livepatch.sh +10 -0
+# - 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.

Quoted 10 lines

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 223
Quoted 6 lines
tools/testing/selftests/livepatch/test-livepatch.sh +6 -0
+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 ...

Quoted 4 lines

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.

Quoted 5 lines

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.)

Quoted 10 lines

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.

Quoted 14 lines

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 237
Quoted 13 lines
tools/testing/selftests/livepatch/test-livepatch.sh +13 -0
+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.)

Quoted 39 lines

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/[email protected]/
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>

Quoted 5 lines

, see https://lore.kernel.org/all/[email protected]/
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?

Quoted 16 lines

tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 250

Quoted 13 lines

tools/testing/selftests/livepatch/test-livepatch.sh +13 -0

+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:

Quoted 8 lines

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

Miroslav Benes

maintainer

2026-03-27 10:46 UTC #7 lore raw

Quoted 39 lines

Quoted 5 lines
Quoted 5 lines
tools/testing/selftests/livepatch/test-livepatch.sh +5 -0
+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"
Quoted 9 lines
tools/testing/selftests/livepatch/test-livepatch.sh livepatch: '$MOD_REPLACE': unpatching complete · line 237
Quoted 8 lines
tools/testing/selftests/livepatch/test-livepatch.sh +8 -0
+	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

Quoted 7 lines

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

Pablo Hugen

author

2026-03-31 21:10 UTC #8 lore raw

Quoted 5 lines

Quoted 5 lines
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).

Quoted 8 lines

Quoted 4 lines
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.

Quoted 2 lines

Quoted 2 lines
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

Petr Mladek

maintainer

2026-03-30 13:44 UTC #9 lore raw

In reply to Joe Lawrence

Quoted 20 lines · Joe Lawrence

Quoted 10 lines · Petr Mladek
Quoted 6 lines · Pablo Hugen
From: Pablo Alessandro Santos Hugen <[email protected]>
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:
Quoted 8 lines
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.

Quoted 3 lines

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

Petr Mladek

maintainer

2026-03-30 13:49 UTC #10 lore raw

speaker-test-module: Re: [PATCH] selftests/livepatch: add test for module function patching

In reply to Joe Lawrence

Quoted 39 lines · Joe Lawrence

Quoted 38 lines · Petr Mladek
Quoted 1 line · Pablo Hugen
From: Pablo Alessandro Santos Hugen <[email protected]>
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/[email protected]/
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>

Quoted 8 lines

Quoted 5 lines
, see https://lore.kernel.org/all/[email protected]/
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