Live Patching
 help / color / mirror / Atom feed
* [PATCH 0/4] selftests: livepatch: Support 4.12 kernels
@ 2026-05-24 23:50 Marcos Paulo de Souza
  2026-05-24 23:50 ` [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function Marcos Paulo de Souza
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-24 23:50 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Shuah Khan
  Cc: live-patching, linux-kselftest, linux-kernel,
	Marcos Paulo de Souza, marcos

One issue that exists since the inception of the livepatch selftests is
the capability of recovering from a failed test: the tested livepatch
modules were kept loaded, interfering with the next tests. The first two
commits introduce a simple mechanism that tracks all loaded modules,
and if the test is aborted, the trap function will disable/unload
the remaining modules. With these changes the test
"livepatch interaction with kprobed function without post_handler" fails
on 4.12 kernels (lacking the feature to have a kprobe a livepatches
to be used alongside), but the loaded modules are unloaded, not
affecting the next tests.

The other patches adapt the test_klp_mod_target.c file to use
module_param_cb and kernel_param_ops structures to test livepatching
of functions in modules. This was done to allow the test to be compiled
on 4.12 kernels. Older kernels lacks proc_create_single function, and
to adapt the code for older versions would require ifdefs, which are
not desirable.

I tested these changes using the current upstream kernel and kernel
from SLE 12-SP5 (kernel 4.12). The test-kprobe.sh test fails on 4.12 due
to the missing capability of livepatch and kprobes to be used together.
The result is that now selftests is able to unload the modules loaded,
and continue with the next tests. For the livepatch target test, it works
the same for 4.12 and current upstream.

Please test and review!

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Marcos Paulo de Souza (4):
      selftests: livepatch: Introduce _remove_mod function
      selftests: livepatch: Remove leftover modules when a testcase fails
      selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels
      selftests: livepatch: Add information about minimum kernel support

 tools/testing/selftests/livepatch/README           |  3 ++
 tools/testing/selftests/livepatch/functions.sh     | 35 ++++++++++++++++++++--
 .../testing/selftests/livepatch/test-livepatch.sh  | 23 +++++++-------
 .../livepatch/test_modules/test_klp_mod_patch.c    | 11 ++++---
 .../livepatch/test_modules/test_klp_mod_target.c   | 22 +++++++-------
 5 files changed, 62 insertions(+), 32 deletions(-)
---
base-commit: 8f7168335cb2e438668c5d94eea76621c9a10edd
change-id: 20260506-livepatch-unload-on-fail-56d30a4e45ca

Best regards,
--  
Marcos Paulo de Souza <mpdesouza@suse.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function
  2026-05-24 23:50 [PATCH 0/4] selftests: livepatch: Support 4.12 kernels Marcos Paulo de Souza
@ 2026-05-24 23:50 ` Marcos Paulo de Souza
  2026-05-24 23:59   ` sashiko-bot
  2026-05-24 23:50 ` [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails Marcos Paulo de Souza
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-24 23:50 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Shuah Khan
  Cc: live-patching, linux-kselftest, linux-kernel,
	Marcos Paulo de Souza, marcos

This new function will be used in the next patch to remove loaded
modules when a testcase fails.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/functions.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 2bc50271729c..3ec0b7962fc5 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -241,9 +241,10 @@ function load_failing_mod() {
 	log "$ret"
 }
 
-# unload_mod(modname) - unload a kernel module
+# _remove_mod(modname) - Internal function to remove a loaded module.
+#                        Use unload_mod() instead, which also updates TEST_MODS tracking.
 #	modname - module name to unload
-function unload_mod() {
+function _remove_mod() {
 	local mod="$1"
 
 	# Wait for module reference count to clear ...
@@ -261,6 +262,14 @@ function unload_mod() {
 		die "failed to unload module $mod (/sys/module)"
 }
 
+# unload_mod(modname) - unload a kernel module
+#	modname - module name to unload
+function unload_mod() {
+	local mod="$1"
+
+	_remove_mod "$mod"
+}
+
 # unload_lp(modname) - unload a kernel module with a livepatch
 #	modname - module name to unload
 function unload_lp() {

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails
  2026-05-24 23:50 [PATCH 0/4] selftests: livepatch: Support 4.12 kernels Marcos Paulo de Souza
  2026-05-24 23:50 ` [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function Marcos Paulo de Souza
@ 2026-05-24 23:50 ` Marcos Paulo de Souza
  2026-05-25  0:06   ` sashiko-bot
  2026-05-24 23:50 ` [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels Marcos Paulo de Souza
  2026-05-24 23:50 ` [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support Marcos Paulo de Souza
  3 siblings, 1 reply; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-24 23:50 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Shuah Khan
  Cc: live-patching, linux-kselftest, linux-kernel,
	Marcos Paulo de Souza, marcos

The current livepatch selftest scripts load modules, run tests and
unloads them. If the test fails, it can leave loaded modules behind, and
in some cases making it impossible to run the next tests.

This approach tracks down the loaded modules, and in case of a test
failure, or premature exit of the script, the cleanup function will
be called by the trap installed on setup_config function.

The cleanup function iterates over the list of leftover loaded modules,
unloading them. The function also checks if the given module is a
livepatch, properly disabling it before unloading.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/functions.sh | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 3ec0b7962fc5..25f137003865 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -15,6 +15,8 @@ if [[ -e /sys/kernel/tracing/trace ]]; then
 else
 	SYSFS_TRACING_DIR="$SYSFS_DEBUG_DIR/tracing"
 fi
+# List of loaded modules used in tests
+TEST_MODS=()
 
 # Kselftest framework requirement - SKIP code is 4
 ksft_skip=4
@@ -125,6 +127,14 @@ function set_ftrace_enabled() {
 }
 
 function cleanup() {
+	# Remove leftover modules in reverse order to handle dependencies
+	for mod_item in "${TEST_MODS[@]}"; do
+		if is_livepatch_mod "$mod_item"; then
+			disable_lp "$mod_item"
+		fi
+		_remove_mod "$mod_item"
+	done
+
 	pop_config
 }
 
@@ -181,6 +191,9 @@ function __load_mod() {
 	# Wait for module in sysfs ...
 	loop_until '[[ -e "/sys/module/$mod" ]]' ||
 		die "failed to load module $mod"
+
+	# Store the module in the modules list
+	TEST_MODS+=("$mod")
 }
 
 
@@ -262,12 +275,20 @@ function _remove_mod() {
 		die "failed to unload module $mod (/sys/module)"
 }
 
-# unload_mod(modname) - unload a kernel module
+# unload_mod(modname) - unload a kernel module and remove it from TEST_MODS
 #	modname - module name to unload
 function unload_mod() {
 	local mod="$1"
 
 	_remove_mod "$mod"
+
+	# Remove from TEST_MODS array
+	for i in "${!TEST_MODS[@]}"; do
+		if [[ "${TEST_MODS[$i]}" == "$mod" ]]; then
+			unset 'TEST_MODS[$i]'
+			break
+		fi
+	done
 }
 
 # unload_lp(modname) - unload a kernel module with a livepatch

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels
  2026-05-24 23:50 [PATCH 0/4] selftests: livepatch: Support 4.12 kernels Marcos Paulo de Souza
  2026-05-24 23:50 ` [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function Marcos Paulo de Souza
  2026-05-24 23:50 ` [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails Marcos Paulo de Souza
@ 2026-05-24 23:50 ` Marcos Paulo de Souza
  2026-05-25  0:24   ` sashiko-bot
  2026-05-24 23:50 ` [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support Marcos Paulo de Souza
  3 siblings, 1 reply; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-24 23:50 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Shuah Khan
  Cc: live-patching, linux-kselftest, linux-kernel,
	Marcos Paulo de Souza, marcos

Use the stable module_param_cb API instead of proc_fs for exposing module
state. This approach is compatible with kernel 4.12 and later. The end
result is the same: the module has a function that shows a string, which
is later livepatched to show a different string. The only difference is
that the file being checked is now a module parameter instead of a
procfs entry.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/functions.sh     |  1 +
 .../testing/selftests/livepatch/test-livepatch.sh  | 23 +++++++++++-----------
 .../livepatch/test_modules/test_klp_mod_patch.c    | 11 +++++------
 .../livepatch/test_modules/test_klp_mod_target.c   | 22 ++++++++++-----------
 4 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index 25f137003865..c8c99851c3a2 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -7,6 +7,7 @@
 MAX_RETRIES=600
 RETRY_INTERVAL=".1"	# seconds
 SYSFS_KERNEL_DIR="/sys/kernel"
+SYSFS_MODULE_DIR="/sys/module"
 SYSFS_KLP_DIR="$SYSFS_KERNEL_DIR/livepatch"
 SYSFS_DEBUG_DIR="$SYSFS_KERNEL_DIR/debug"
 SYSFS_KPROBES_DIR="$SYSFS_DEBUG_DIR/kprobes"
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index c44c5341a2f1..06aacf0f4dc7 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -198,26 +198,25 @@ 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
+# - load a target module with module_param_cb get data with original output
+# - load a livepatch that patches the target module's get function
+# - verify the parameter get function returns the livepatched output
 # - disable and unload the livepatch
-# - verify the proc entry returns original output again
+# - verify the parameter get function returns the 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
+if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" != "$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
+if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
 	echo -e "FAIL\n\n"
 	die "livepatch kselftest(s) failed"
 fi
@@ -225,7 +224,7 @@ fi
 disable_lp $MOD_TARGET_PATCH
 unload_lp $MOD_TARGET_PATCH
 
-if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" != "$MOD_TARGET: original output" ]] ; then
 	echo -e "FAIL\n\n"
 	die "livepatch kselftest(s) failed"
 fi
@@ -252,9 +251,9 @@ $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
+# - verify the parameter get function returns the livepatched output
 # - disable and unload the livepatch
-# - verify the proc entry returns original output again
+# - verify the parameter get function returns the original output again
 # - unload the target module
 
 start_test "module function patching (livepatch first)"
@@ -262,7 +261,7 @@ 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
+if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" != "$MOD_TARGET_PATCH: this has been live patched" ]] ; then
 	echo -e "FAIL\n\n"
 	die "livepatch kselftest(s) failed"
 fi
@@ -270,7 +269,7 @@ fi
 disable_lp $MOD_TARGET_PATCH
 unload_lp $MOD_TARGET_PATCH
 
-if [[ "$(cat /proc/$MOD_TARGET)" != "$MOD_TARGET: original output" ]] ; then
+if [[ "$(cat $SYSFS_MODULE_DIR/$MOD_TARGET/parameters/klp_mod_arg)" != "$MOD_TARGET: original output" ]] ; then
 	echo -e "FAIL\n\n"
 	die "livepatch kselftest(s) failed"
 fi
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
index 6725b4720365..ab56e57c02a8 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_patch.c
@@ -8,17 +8,16 @@
 #include <linux/livepatch.h>
 #include <linux/seq_file.h>
 
-static int livepatch_mod_target_show(struct seq_file *m, void *v)
+static int livepatch_mod_target_get(char *buffer, const struct kernel_param *kp)
 {
-	seq_printf(m, "%s: %s\n", THIS_MODULE->name,
-		   "this has been live patched");
-	return 0;
+	return sprintf(buffer, "%s: %s\n", THIS_MODULE->name,
+		       "this has been live patched");
 }
 
 static struct klp_func funcs[] = {
 	{
-		.old_name = "test_klp_mod_target_show",
-		.new_func = livepatch_mod_target_show,
+		.old_name = "test_klp_mod_target_get",
+		.new_func = livepatch_mod_target_get,
 	},
 	{},
 };
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
index 9643984d2402..1382a266825f 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
@@ -5,31 +5,29 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
+#include <linux/moduleparam.h>
 
-static struct proc_dir_entry *pde;
-
-static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
+static noinline int test_klp_mod_target_get(char *buffer, const struct kernel_param *kp)
 {
-	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
-	return 0;
+	return sprintf(buffer, "%s: %s\n", THIS_MODULE->name, "original output");
 }
 
+static const struct kernel_param_ops test_klp_mod_target_ops = {
+	.get = test_klp_mod_target_get,
+};
+
+module_param_cb(klp_mod_arg, &test_klp_mod_target_ops, NULL, 0444);
+MODULE_PARM_DESC(klp_mod_arg, "The value of this argument will be livepatched");
+
 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);

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support
  2026-05-24 23:50 [PATCH 0/4] selftests: livepatch: Support 4.12 kernels Marcos Paulo de Souza
                   ` (2 preceding siblings ...)
  2026-05-24 23:50 ` [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels Marcos Paulo de Souza
@ 2026-05-24 23:50 ` Marcos Paulo de Souza
  2026-05-25  0:29   ` sashiko-bot
  3 siblings, 1 reply; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-24 23:50 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Shuah Khan
  Cc: live-patching, linux-kselftest, linux-kernel,
	Marcos Paulo de Souza, marcos

The current livepatch selftests are compatible with kernel 4.12, so add
a note about it for future developers willing to contribute with new
tests.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/README | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/livepatch/README b/tools/testing/selftests/livepatch/README
index d2035dd64a2b..293f4730b927 100644
--- a/tools/testing/selftests/livepatch/README
+++ b/tools/testing/selftests/livepatch/README
@@ -54,3 +54,6 @@ check_result().  The latter function greps the kernel's ring buffer for
 those strings for result comparison.  Other utility functions include
 general module loading and livepatch loading helpers (waiting for patch
 transitions, sysfs entries, etc.)
+
+All new tests and test modules MUST be compatible with kernel version 4.12 and
+later, including current upstream kernels.

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function
  2026-05-24 23:50 ` [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function Marcos Paulo de Souza
@ 2026-05-24 23:59   ` sashiko-bot
  2026-05-25 11:49     ` Marcos Paulo de Souza
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-05-24 23:59 UTC (permalink / raw)
  To: Jiri Kosina, Petr Mladek, Joe Lawrence, Shuah Khan,
	Miroslav Benes, Josh Poimboeuf
  Cc: linux-kernel, linux-kselftest, marcos, live-patching,
	Marcos Paulo de Souza

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `_remove_mod` uses `die()` which will terminate the script during cleanup operations.
--

commit 7b1591f4e5c3b351136e7fe2821eb2e15ae59ad0
Author: Marcos Paulo de Souza <mpdesouza@suse.com>

selftests: livepatch: Introduce _remove_mod function

This commit introduces the _remove_mod internal function to remove a loaded
module, intended to be used during cleanup when a testcase fails. It also
refactors unload_mod to use this new helper function.

> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index 2bc50271729c6..3ec0b7962fc5c 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh

[ ... ]

> @@ -241,9 +241,10 @@ function load_failing_mod() {
>  	log "$ret"
>  }
>  
> -# unload_mod(modname) - unload a kernel module
> +# _remove_mod(modname) - Internal function to remove a loaded module.
> +#                        Use unload_mod() instead, which also updates TEST_MODS tracking.
>  #	modname - module name to unload
> -function unload_mod() {
> +function _remove_mod() {
>  	local mod="$1"
>  
>  	# Wait for module reference count to clear ...
> @@ -261,6 +262,14 @@ function unload_mod() {
>  		die "failed to unload module $mod (/sys/module)"

[Severity: Medium]
The commit message notes this function will be used to remove loaded modules
when a testcase fails. If used in a cleanup path (such as a bash EXIT trap),
will calling die() here terminate the script prematurely?

If a module fails to unload (for example, if its refcount doesn't drop),
die() executes exit 1. Inside a trap, this aborts the rest of the
cleanup process. This could skip unloading any remaining modules and
prevent final cleanup operations like pop_config() from running, potentially
leaving kernel tracing persistently enabled and polluting the system state
for subsequent tests.

>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524-livepatch-unload-on-fail-v1-0-7465de7f741d@suse.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails
  2026-05-24 23:50 ` [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails Marcos Paulo de Souza
@ 2026-05-25  0:06   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-25  0:06 UTC (permalink / raw)
  To: Josh Poimboeuf, Joe Lawrence, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Shuah Khan
  Cc: Marcos Paulo de Souza, linux-kselftest, marcos, live-patching,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The cleanup loop iterates over the TEST_MODS array in forward order, violating dependency requirements and causing rmmod to fail.
- [Medium] Functions invoked during cleanup call die() on failure, prematurely aborting the EXIT trap and skipping pop_config().
--

commit aafb48b2a54fe2209742378f31fb96583598557a
Author: Marcos Paulo de Souza <mpdesouza@suse.com>

selftests: livepatch: Remove leftover modules when a testcase fails

This commit introduces a cleanup mechanism for livepatch selftests that
tracks loaded modules and attempts to remove them if a test fails or exits
prematurely. It uses a TEST_MODS array to keep track of dependencies and
unloads the remaining modules during an EXIT trap.

> diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
> index 3ec0b7962fc5c..25f1370038654 100644
> --- a/tools/testing/selftests/livepatch/functions.sh
> +++ b/tools/testing/selftests/livepatch/functions.sh
[ ... ]
> @@ -125,6 +127,14 @@ function set_ftrace_enabled() {
>  }
>  
>  function cleanup() {
> +	# Remove leftover modules in reverse order to handle dependencies
> +	for mod_item in "${TEST_MODS[@]}"; do

[Severity: Medium]
Does this loop actually iterate in reverse order?

The comment states the modules will be removed in reverse order to handle
dependencies, but ${TEST_MODS[@]} expands the array in ascending index
order.

Because dependent modules (like livepatches) are loaded after their base
modules, iterating in forward order will attempt to unload the base modules
first. This would likely cause rmmod to fail with a module in use error.

> +		if is_livepatch_mod "$mod_item"; then
> +			disable_lp "$mod_item"
> +		fi
> +		_remove_mod "$mod_item"

[Severity: Medium]
Can this call prematurely abort the cleanup process?

The cleanup() function is executed within an EXIT trap. If _remove_mod()
fails to unload a module, it calls die() which executes an exit command:

tools/testing/selftests/livepatch/functions.sh:_remove_mod() {
    ...
    die "failed to unload module $mod (/sys/module)"
}

Executing exit inside a bash EXIT trap immediately halts the execution of
the trap itself. This would abruptly skip the remaining iterations of the
module cleanup loop and skip the subsequent pop_config() call.

Failing to execute pop_config() leaves the global kernel debug and tracing
configurations modified, which could pollute the state of the test system.

> +	done
> +
>  	pop_config
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524-livepatch-unload-on-fail-v1-0-7465de7f741d@suse.com?part=2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels
  2026-05-24 23:50 ` [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels Marcos Paulo de Souza
@ 2026-05-25  0:24   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-25  0:24 UTC (permalink / raw)
  To: Miroslav Benes, Shuah Khan, Petr Mladek, Josh Poimboeuf,
	Joe Lawrence, Jiri Kosina
  Cc: live-patching, linux-kselftest, Marcos Paulo de Souza, marcos,
	linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `test_klp_mod_target_ops` structure omits the logically required `.set` callback, exposing the kernel to a NULL pointer dereference during module load.
--

commit 532c1b8c7438f534fd4e3440716ede6cf83efe1c
Author: Marcos Paulo de Souza <mpdesouza@suse.com>

selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels

Use the stable module_param_cb API instead of proc_fs for exposing module
state. This approach is compatible with kernel 4.12 and later.

> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> index 9643984d24022..1382a266825fb 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> @@ -5,31 +5,29 @@
[ ... ]
> +static const struct kernel_param_ops test_klp_mod_target_ops = {
> +	.get = test_klp_mod_target_get,
> +};
> +
> +module_param_cb(klp_mod_arg, &test_klp_mod_target_ops, NULL, 0444);

[Severity: Medium]
Does this code need a .set callback in the kernel_param_ops structure?

When a module parameter is passed during module load, the kernel parses the
arguments using parse_args(), which invokes parse_one() in kernel/params.c.
Because parse_one() unconditionally executes the .set callback without
verifying if it is actually defined, omitting it here leaves the callback
implicitly initialized to NULL.

Could this lead to a NULL pointer dereference and a kernel panic if someone
attempts to set this parameter during module load?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524-livepatch-unload-on-fail-v1-0-7465de7f741d@suse.com?part=3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support
  2026-05-24 23:50 ` [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support Marcos Paulo de Souza
@ 2026-05-25  0:29   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-25  0:29 UTC (permalink / raw)
  To: Miroslav Benes, Jiri Kosina, Shuah Khan, Josh Poimboeuf,
	Petr Mladek, Joe Lawrence
  Cc: linux-kselftest, linux-kernel, live-patching,
	Marcos Paulo de Souza, marcos

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mandating backward compatibility with an ancient kernel version (4.12) for all new in-tree selftests violates established upstream kernel principles.
--

commit 005a4d6b98d5e0ddf7ab7d595575364d010c4d3f
Author: Marcos Paulo de Souza <mpdesouza@suse.com>

selftests: livepatch: Add information about minimum kernel support

This commit adds a note to the livepatch selftests README. It documents that
the current tests are compatible with kernel 4.12 and mandates that all new
tests must maintain this backward compatibility.

> diff --git a/tools/testing/selftests/livepatch/README b/tools/testing/selftests/livepatch/README
> index d2035dd64a2be..293f4730b9277 100644
> --- a/tools/testing/selftests/livepatch/README
> +++ b/tools/testing/selftests/livepatch/README
> @@ -54,3 +54,6 @@ check_result().  The latter function greps the kernel's ring buffer for
>  those strings for result comparison.  Other utility functions include
>  general module loading and livepatch loading helpers (waiting for patch
>  transitions, sysfs entries, etc.)
> +
> +All new tests and test modules MUST be compatible with kernel version 4.12 and
> +later, including current upstream kernels.

[Severity: Medium]
Does mandating backward compatibility with kernel 4.12 for all new tests
conflict with upstream kernel principles?

Requiring tests to support an older kernel could block the introduction of
tests for any livepatch features developed after 4.12, as the required
internal APIs would not exist. This might also force developers to clutter
the test module codebase with version checks for old kernels.

Should the burden of backporting tests to older enterprise kernels be left
to downstream distributions rather than enforced on upstream contributors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524-livepatch-unload-on-fail-v1-0-7465de7f741d@suse.com?part=4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function
  2026-05-24 23:59   ` sashiko-bot
@ 2026-05-25 11:49     ` Marcos Paulo de Souza
  0 siblings, 0 replies; 10+ messages in thread
From: Marcos Paulo de Souza @ 2026-05-25 11:49 UTC (permalink / raw)
  To: sashiko-reviews, Jiri Kosina, Petr Mladek, Joe Lawrence,
	Shuah Khan, Miroslav Benes, Josh Poimboeuf
  Cc: linux-kernel, linux-kselftest, live-patching

On Sun, 2026-05-24 at 23:59 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] `_remove_mod` uses `die()` which will terminate the script
> during cleanup operations.
> --
> 
> commit 7b1591f4e5c3b351136e7fe2821eb2e15ae59ad0
> Author: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> selftests: livepatch: Introduce _remove_mod function
> 
> This commit introduces the _remove_mod internal function to remove a
> loaded
> module, intended to be used during cleanup when a testcase fails. It
> also
> refactors unload_mod to use this new helper function.
> 
> > diff --git a/tools/testing/selftests/livepatch/functions.sh
> > b/tools/testing/selftests/livepatch/functions.sh
> > index 2bc50271729c6..3ec0b7962fc5c 100644
> > --- a/tools/testing/selftests/livepatch/functions.sh
> > +++ b/tools/testing/selftests/livepatch/functions.sh
> 
> [ ... ]
> 
> > @@ -241,9 +241,10 @@ function load_failing_mod() {
> >  	log "$ret"
> >  }
> >  
> > -# unload_mod(modname) - unload a kernel module
> > +# _remove_mod(modname) - Internal function to remove a loaded
> > module.
> > +#                        Use unload_mod() instead, which also
> > updates TEST_MODS tracking.
> >  #	modname - module name to unload
> > -function unload_mod() {
> > +function _remove_mod() {
> >  	local mod="$1"
> >  
> >  	# Wait for module reference count to clear ...
> > @@ -261,6 +262,14 @@ function unload_mod() {
> >  		die "failed to unload module $mod (/sys/module)"
> 
> [Severity: Medium]
> The commit message notes this function will be used to remove loaded
> modules
> when a testcase fails. If used in a cleanup path (such as a bash EXIT
> trap),
> will calling die() here terminate the script prematurely?
> 
> If a module fails to unload (for example, if its refcount doesn't
> drop),
> die() executes exit 1. Inside a trap, this aborts the rest of the
> cleanup process. This could skip unloading any remaining modules and
> prevent final cleanup operations like pop_config() from running,
> potentially
> leaving kernel tracing persistently enabled and polluting the system
> state
> for subsequent tests.

The system is already polluted when a testcase fails nowadays, since a
module is kept loaded either way, making the next tests to fail if they
also try to lead the same modules.

The current approach is trying to make it more reliable.

> 
> >  }
> 
> [ ... ]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-25 11:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-24 23:50 [PATCH 0/4] selftests: livepatch: Support 4.12 kernels Marcos Paulo de Souza
2026-05-24 23:50 ` [PATCH 1/4] selftests: livepatch: Introduce _remove_mod function Marcos Paulo de Souza
2026-05-24 23:59   ` sashiko-bot
2026-05-25 11:49     ` Marcos Paulo de Souza
2026-05-24 23:50 ` [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails Marcos Paulo de Souza
2026-05-25  0:06   ` sashiko-bot
2026-05-24 23:50 ` [PATCH 3/4] selftests: livepatch: Adapt mod_target module to pass on 4.12 kernels Marcos Paulo de Souza
2026-05-25  0:24   ` sashiko-bot
2026-05-24 23:50 ` [PATCH 4/4] selftests: livepatch: Add information about minimum kernel support Marcos Paulo de Souza
2026-05-25  0:29   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox