From: Yafang Shao <laoar.shao@gmail.com>
To: jpoimboe@kernel.org, jikos@kernel.org, mbenes@suse.cz,
pmladek@suse.com, joe.lawrence@redhat.com, mcgrof@kernel.org
Cc: live-patching@vger.kernel.org, linux-modules@vger.kernel.org,
Yafang Shao <laoar.shao@gmail.com>
Subject: [PATCH v2 2/2] livepatch: Delete the associated module of disabled livepatch
Date: Sun, 7 Apr 2024 11:57:30 +0800 [thread overview]
Message-ID: <20240407035730.20282-3-laoar.shao@gmail.com> (raw)
In-Reply-To: <20240407035730.20282-1-laoar.shao@gmail.com>
In our production environment, upon loading a new atomic replace livepatch,
we encountered an issue where the kernel module of the old livepatch
remained, despite being replaced by the new one. This issue can be
reproduced using the following steps:
- Pre setup: Build two atomic replace livepatches
- First step: Load the first livepatch using the insmod command
$ insmod livepatch-test_0.ko
$ ls /sys/kernel/livepatch/
livepatch_test_0
$ cat /sys/kernel/livepatch/livepatch_test_0/enabled
1
$ lsmod | grep livepatch
livepatch_test_0 16384 1
- Second step: Load the new livepatch using the insmod command
$ insmod livepatch-test_1.ko
$ ls /sys/kernel/livepatch/
livepatch_test_1 <<<< livepatch_test_0 was replaced
$ cat /sys/kernel/livepatch/livepatch_test_1/enabled
1
$ lsmod | grep livepatch
livepatch_test_1 16384 1
livepatch_test_0 16384 0 <<<< leftover
Detecting which livepatch will be replaced by the new one from userspace is
not reliable, necessitating the need for the operation to be performed
within the kernel itself. With this improvement, executing
`insmod livepatch-test_1.ko` will automatically remove the
livepatch-test_0.ko module.
Following this change, the associated kernel module will be removed when
executing `echo 0 > /sys/kernel/livepatch/${livepath}/enabled`. Therefore,
adjustments need to be made to the selftests accordingly.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/livepatch/core.c | 16 +++++++++----
.../testing/selftests/livepatch/functions.sh | 2 ++
.../selftests/livepatch/test-callbacks.sh | 24 +++++--------------
.../selftests/livepatch/test-ftrace.sh | 3 +--
.../selftests/livepatch/test-livepatch.sh | 11 +++------
.../testing/selftests/livepatch/test-state.sh | 15 ++++--------
.../selftests/livepatch/test-syscall.sh | 3 +--
.../testing/selftests/livepatch/test-sysfs.sh | 6 ++---
8 files changed, 30 insertions(+), 50 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index ecbc9b6aba3a..6850158bcef4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -709,8 +709,10 @@ static void klp_free_patch_start(struct klp_patch *patch)
* the last function accessing the livepatch structures when the patch
* gets disabled.
*/
-static void klp_free_patch_finish(struct klp_patch *patch)
+static void klp_free_patch_finish(struct klp_patch *patch, bool delete)
{
+ struct module *mod = patch->mod;
+
/*
* Avoid deadlock with enabled_store() sysfs callback by
* calling this outside klp_mutex. It is safe because
@@ -721,8 +723,12 @@ static void klp_free_patch_finish(struct klp_patch *patch)
wait_for_completion(&patch->finish);
/* Put the module after the last access to struct klp_patch. */
- if (!patch->forced)
- module_put(patch->mod);
+ if (!patch->forced) {
+ module_put(mod);
+ if (!delete)
+ return;
+ delete_module(mod);
+ }
}
/*
@@ -735,7 +741,7 @@ static void klp_free_patch_work_fn(struct work_struct *work)
struct klp_patch *patch =
container_of(work, struct klp_patch, free_work);
- klp_free_patch_finish(patch);
+ klp_free_patch_finish(patch, true);
}
void klp_free_patch_async(struct klp_patch *patch)
@@ -1124,7 +1130,7 @@ int klp_enable_patch(struct klp_patch *patch)
mutex_unlock(&klp_mutex);
- klp_free_patch_finish(patch);
+ klp_free_patch_finish(patch, false);
return ret;
}
diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index fc4c6a016d38..21c48f9b020e 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -220,6 +220,8 @@ function load_failing_mod() {
function unload_mod() {
local mod="$1"
+ [[ ! -d "/sys/module/$mod" ]] && return
+
# Wait for module reference count to clear ...
loop_until '[[ $(cat "/sys/module/$mod/refcnt") == "0" ]]' ||
die "failed to unload module $mod (refcnt)"
diff --git a/tools/testing/selftests/livepatch/test-callbacks.sh b/tools/testing/selftests/livepatch/test-callbacks.sh
index 32b150e25b10..833cc66915f7 100755
--- a/tools/testing/selftests/livepatch/test-callbacks.sh
+++ b/tools/testing/selftests/livepatch/test-callbacks.sh
@@ -55,7 +55,6 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] Normal state
livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
% rmmod $MOD_TARGET
$MOD_TARGET: ${MOD_TARGET}_exit"
@@ -103,7 +102,6 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET -> [MODULE_STATE_LIVE] Normal state
livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
% rmmod $MOD_TARGET
$MOD_TARGET: ${MOD_TARGET}_exit"
@@ -152,8 +150,7 @@ $MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# This test is similar to the previous test, however the livepatch is
@@ -201,8 +198,7 @@ $MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# A simple test of loading a livepatch without one of its patch target
@@ -233,8 +229,7 @@ $MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# Test a scenario where a vmlinux pre-patch callback returns a non-zero
@@ -316,8 +311,7 @@ $MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# Test loading multiple targeted kernel modules. This test-case is
@@ -373,7 +367,6 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
% rmmod $MOD_TARGET_BUSY
$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
@@ -445,7 +438,6 @@ livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH: post_unpatch_callback: $MOD_TARGET_BUSY -> [MODULE_STATE_LIVE] Normal state
livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH
% rmmod $MOD_TARGET_BUSY
$MOD_TARGET_BUSY: busymod_work_func exit
$MOD_TARGET_BUSY: ${MOD_TARGET_BUSY}_exit"
@@ -496,9 +488,7 @@ $MOD_LIVEPATCH: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# Load multiple livepatches, but the second as an 'atomic-replace'
@@ -545,9 +535,7 @@ $MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH2': unpatching complete"
exit 0
diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
index 730218bce99c..81539fe3c082 100755
--- a/tools/testing/selftests/livepatch/test-ftrace.sh
+++ b/tools/testing/selftests/livepatch/test-ftrace.sh
@@ -57,8 +57,7 @@ livepatch: sysctl: setting key \"kernel.ftrace_enabled\": Device or resource bus
livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
exit 0
diff --git a/tools/testing/selftests/livepatch/test-livepatch.sh b/tools/testing/selftests/livepatch/test-livepatch.sh
index e3455a6b1158..8209f4f239fb 100755
--- a/tools/testing/selftests/livepatch/test-livepatch.sh
+++ b/tools/testing/selftests/livepatch/test-livepatch.sh
@@ -41,8 +41,7 @@ livepatch: '$MOD_LIVEPATCH': patching complete
livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# - load a livepatch that modifies the output from /proc/cmdline and
@@ -95,14 +94,12 @@ livepatch: '$MOD_REPLACE': initializing unpatching transition
livepatch: '$MOD_REPLACE': starting unpatching transition
livepatch: '$MOD_REPLACE': completing unpatching transition
livepatch: '$MOD_REPLACE': unpatching complete
-% rmmod $MOD_REPLACE
$MOD_LIVEPATCH: this has been live patched
% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH/enabled
livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# - load a livepatch that modifies the output from /proc/cmdline and
@@ -149,14 +146,12 @@ livepatch: '$MOD_REPLACE': starting patching transition
livepatch: '$MOD_REPLACE': completing patching transition
livepatch: '$MOD_REPLACE': patching complete
$MOD_REPLACE: this has been live patched
-% rmmod $MOD_LIVEPATCH
$MOD_REPLACE: this has been live patched
% echo 0 > /sys/kernel/livepatch/$MOD_REPLACE/enabled
livepatch: '$MOD_REPLACE': initializing unpatching transition
livepatch: '$MOD_REPLACE': starting unpatching transition
livepatch: '$MOD_REPLACE': completing unpatching transition
-livepatch: '$MOD_REPLACE': unpatching complete
-% rmmod $MOD_REPLACE"
+livepatch: '$MOD_REPLACE': unpatching complete"
exit 0
diff --git a/tools/testing/selftests/livepatch/test-state.sh b/tools/testing/selftests/livepatch/test-state.sh
index 10a52ac06185..2cc0ef3752c2 100755
--- a/tools/testing/selftests/livepatch/test-state.sh
+++ b/tools/testing/selftests/livepatch/test-state.sh
@@ -37,8 +37,7 @@ livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
$MOD_LIVEPATCH: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH: free_loglevel_state: freeing space for the stored console_loglevel
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
# Take over system state change by a cumulative patch
@@ -71,7 +70,6 @@ livepatch: '$MOD_LIVEPATCH2': completing patching transition
$MOD_LIVEPATCH2: post_patch_callback: vmlinux
$MOD_LIVEPATCH2: fix_console_loglevel: taking over the console_loglevel change
livepatch: '$MOD_LIVEPATCH2': patching complete
-% rmmod $MOD_LIVEPATCH
% echo 0 > /sys/kernel/livepatch/$MOD_LIVEPATCH2/enabled
livepatch: '$MOD_LIVEPATCH2': initializing unpatching transition
$MOD_LIVEPATCH2: pre_unpatch_callback: vmlinux
@@ -80,8 +78,7 @@ livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2"
+livepatch: '$MOD_LIVEPATCH2': unpatching complete"
# Take over system state change by a cumulative patch
@@ -116,7 +113,6 @@ livepatch: '$MOD_LIVEPATCH3': completing patching transition
$MOD_LIVEPATCH3: post_patch_callback: vmlinux
$MOD_LIVEPATCH3: fix_console_loglevel: taking over the console_loglevel change
livepatch: '$MOD_LIVEPATCH3': patching complete
-% rmmod $MOD_LIVEPATCH2
% insmod test_modules/$MOD_LIVEPATCH2.ko
livepatch: enabling patch '$MOD_LIVEPATCH2'
livepatch: '$MOD_LIVEPATCH2': initializing patching transition
@@ -135,9 +131,7 @@ livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2
-% rmmod $MOD_LIVEPATCH3"
+livepatch: '$MOD_LIVEPATCH2': unpatching complete"
# Failure caused by incompatible cumulative livepatches
@@ -170,7 +164,6 @@ livepatch: '$MOD_LIVEPATCH2': starting unpatching transition
livepatch: '$MOD_LIVEPATCH2': completing unpatching transition
$MOD_LIVEPATCH2: post_unpatch_callback: vmlinux
$MOD_LIVEPATCH2: free_loglevel_state: freeing space for the stored console_loglevel
-livepatch: '$MOD_LIVEPATCH2': unpatching complete
-% rmmod $MOD_LIVEPATCH2"
+livepatch: '$MOD_LIVEPATCH2': unpatching complete"
exit 0
diff --git a/tools/testing/selftests/livepatch/test-syscall.sh b/tools/testing/selftests/livepatch/test-syscall.sh
index b76a881d4013..a4e26cb7f524 100755
--- a/tools/testing/selftests/livepatch/test-syscall.sh
+++ b/tools/testing/selftests/livepatch/test-syscall.sh
@@ -47,7 +47,6 @@ $MOD_SYSCALL: Remaining not livepatched processes: 0
livepatch: '$MOD_SYSCALL': initializing unpatching transition
livepatch: '$MOD_SYSCALL': starting unpatching transition
livepatch: '$MOD_SYSCALL': completing unpatching transition
-livepatch: '$MOD_SYSCALL': unpatching complete
-% rmmod $MOD_SYSCALL"
+livepatch: '$MOD_SYSCALL': unpatching complete"
exit 0
diff --git a/tools/testing/selftests/livepatch/test-sysfs.sh b/tools/testing/selftests/livepatch/test-sysfs.sh
index 6c646afa7395..9adfcead84f6 100755
--- a/tools/testing/selftests/livepatch/test-sysfs.sh
+++ b/tools/testing/selftests/livepatch/test-sysfs.sh
@@ -37,8 +37,7 @@ livepatch: '$MOD_LIVEPATCH': patching complete
livepatch: '$MOD_LIVEPATCH': initializing unpatching transition
livepatch: '$MOD_LIVEPATCH': starting unpatching transition
livepatch: '$MOD_LIVEPATCH': completing unpatching transition
-livepatch: '$MOD_LIVEPATCH': unpatching complete
-% rmmod $MOD_LIVEPATCH"
+livepatch: '$MOD_LIVEPATCH': unpatching complete"
start_test "sysfs test object/patched"
@@ -80,7 +79,6 @@ test_klp_callbacks_demo: pre_unpatch_callback: vmlinux
livepatch: 'test_klp_callbacks_demo': starting unpatching transition
livepatch: 'test_klp_callbacks_demo': completing unpatching transition
test_klp_callbacks_demo: post_unpatch_callback: vmlinux
-livepatch: 'test_klp_callbacks_demo': unpatching complete
-% rmmod test_klp_callbacks_demo"
+livepatch: 'test_klp_callbacks_demo': unpatching complete"
exit 0
--
2.39.1
next prev parent reply other threads:[~2024-04-07 3:58 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-07 3:57 [PATCH v2 0/2] livepatch, module: Delete the associated module of disabled livepatch Yafang Shao
2024-04-07 3:57 ` [PATCH v2 1/2] module: Add a new helper delete_module() Yafang Shao
2024-04-24 12:09 ` Yafang Shao
2024-05-04 16:53 ` Greg KH
2024-05-04 22:26 ` Josh Poimboeuf
2024-05-04 21:36 ` Luis Chamberlain
2024-05-06 11:58 ` Petr Mladek
2024-04-07 3:57 ` Yafang Shao [this message]
2024-05-03 21:14 ` [PATCH v2 2/2] livepatch: Delete the associated module of disabled livepatch Josh Poimboeuf
2024-05-06 11:32 ` Petr Mladek
2024-05-07 2:35 ` Josh Poimboeuf
2024-05-07 14:03 ` Yafang Shao
2024-05-08 5:16 ` Josh Poimboeuf
2024-05-08 6:01 ` Yafang Shao
2024-05-08 7:03 ` Josh Poimboeuf
2024-05-09 2:17 ` Yafang Shao
2024-05-09 5:20 ` Josh Poimboeuf
2024-05-09 5:53 ` Yafang Shao
2024-05-09 10:16 ` Petr Mladek
2024-05-09 11:57 ` Yafang Shao
2024-05-09 10:51 ` Petr Mladek
2024-05-06 12:20 ` Petr Mladek
2024-05-02 13:30 ` [PATCH v2 0/2] livepatch, module: " Joe Lawrence
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240407035730.20282-3-laoar.shao@gmail.com \
--to=laoar.shao@gmail.com \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@kernel.org \
--cc=linux-modules@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=mcgrof@kernel.org \
--cc=pmladek@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).