Re: [PATCH v5 9/9] selftests: livepatch: Add test for function conflict across provides
From: sashiko-bot@kernel.org
Date: 2026-08-09 09:49:45
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Malformed livepatch module missing `old_name` causes a kernel panic on load. Pre-existing issues: - [Medium] Race condition in sysfs polling causes flaky selftests. --
--- Patch [9]: [PATCH v5 9/9] selftests: livepatch: Add test for function conflict across provides ---commit eaca609db12a6842c4e94debcf9d9ef5049826db
Author: Yafang Shao [off-list ref]
selftests: livepatch: Add test for function conflict across provides
This commit adds a new test module test_klp_provides.c and three new test
scenarios to test-provides-obsoletes.sh. These tests verify the behavior
of livepatch function conflicts and replacement when using different
provides IDs with and without obsoletes.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/livepatch/test-provides-obsoletes.sh b/tools/testing/selftests/livepatch/test-provides-obsoletes.sh index 1b2c73bdd50b0..32885625401d1 100755 --- a/tools/testing/selftests/livepatch/test-provides-obsoletes.sh +++ b/tools/testing/selftests/livepatch/test-provides-obsoletes.sh
[ ... ]
quoted hunk ↗ jump to hunk
@@ -278,4 +279,123 @@ $MOD_STATE2: free_loglevel_state: freeing space for the stored console_loglevel livepatch: '$MOD_STATE2': unpatching complete % rmmod $MOD_STATE2" + +# - load a livepatch with provides=1 that modifies meminfo_proc_show +# - try to load another livepatch with provides=2 that modifies the +# same function. The second one must be rejected because livepatches +# with different provides ids and no obsoletes must not modify +# the same function. +# - disable and unload the remaining livepatch + +start_test "function conflict across provides" + +load_lp $MOD_ATOMIC provides=1 +load_failing_mod $MOD_PROVIDES provides=2 + +disable_lp $MOD_ATOMIC +unload_lp $MOD_ATOMIC + +check_result "% insmod test_modules/$MOD_ATOMIC.ko provides=1 +livepatch: enabling patch '$MOD_ATOMIC' +livepatch: '$MOD_ATOMIC': initializing patching transition +livepatch: '$MOD_ATOMIC': starting patching transition +livepatch: '$MOD_ATOMIC': completing patching transition +livepatch: '$MOD_ATOMIC': patching complete +% insmod test_modules/$MOD_PROVIDES.ko provides=2 +livepatch: Livepatch patch ($MOD_PROVIDES) is not compatible with the already installed livepatches. +insmod: ERROR: could not insert module test_modules/$MOD_PROVIDES.ko: Invalid parameters +% echo 0 > $SYSFS_KLP_DIR/$MOD_ATOMIC/enabled +livepatch: '$MOD_ATOMIC': initializing unpatching transition +livepatch: '$MOD_ATOMIC': starting unpatching transition +livepatch: '$MOD_ATOMIC': completing unpatching transition +livepatch: '$MOD_ATOMIC': unpatching complete +% rmmod $MOD_ATOMIC" + + +# - load a livepatch with provides=1 that modifies meminfo_proc_show +# - load another livepatch with provides=1 that modifies the same +# function. The second one loads successfully because livepatches +# with the same provides id replace each other. +# - disable and unload the remaining livepatch + +start_test "function replace within same provides" + +load_lp $MOD_ATOMIC provides=1 +load_lp $MOD_PROVIDES provides=1 + +mods=($SYSFS_KLP_DIR/*) +nmods=${#mods[@]} +if [[ "$nmods" -ne 1 ]]; then
[Severity: Medium] This is a pre-existing issue, but does this pattern cause a regression in the form of flaky selftests? The script synchronously counts the directories in the sysfs path immediately after a transition. However, the old patch's sysfs directory removal is deferred to an asynchronous workqueue: kernel/livepatch/core.c:klp_free_replaced_patches_async() Because of this, the script may momentarily see 2 directories instead of 1 before the asynchronous cleanup completes.
+ die "Expecting one module listed, found $nmods" +fi
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_provides.c b/tools/testing/selftests/livepatch/test_modules/test_klp_provides.c new file mode 100644 index 0000000000000..9751a6f6c8518 --- /dev/null +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_provides.c@@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0 +
[ ... ]
+static int test_klp_provides_init(void)
+{
+#ifdef KLP_HAS_REPLACE
+ patch.replace = replace;
+#else
+ patch.provides = provides;
+ if (nr_obsoletes > 0) {
+ patch.obsoletes = obsoletes;
+ patch.nr_obsoletes = nr_obsoletes;
+ }
+#endif
+ return klp_enable_patch(&patch);
[Severity: High]
Does klp_enable_patch() introduce a regression where a malformed livepatch
module missing old_name causes a kernel panic on load?
If a livepatch module defines a klp_func with .old_name = NULL but provides
.new_func, it bypasses the klp_for_each_func_static loop termination
condition.
Since klp_is_patch_compatible() executes before klp_init_patch():
kernel/livepatch/core.c:klp_enable_patch() {
...
if (!klp_is_patch_compatible(patch)) {
...
ret = klp_init_patch(patch);
...
}
klp_has_function_conflict() iterates over this malformed function, which then
passes the malformed func to klp_find_func():
kernel/livepatch/state.c:klp_has_function_conflict() {
...
if (klp_find_func(old_obj, func))
...
}
This then passes func->old_name to strcmp():
kernel/livepatch/core.c:klp_find_func() {
...
if ((strcmp(obj_func->old_name, func->old_name) == 0) &&
...
}
resulting in an immediate null pointer dereference.
+}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260809091954.22930-1-laoar.shao@gmail.com?part=9