[prev in list] [next in list] [prev in thread] [next in thread] 

List:       live-patching
Subject:    [PATCH v2 2/2] livepatch: Delete the associated module of disabled livepatch
From:       Yafang Shao <laoar.shao () gmail ! com>
Date:       2024-04-07 3:57:30
Message-ID: 20240407035730.20282-3-laoar.shao () gmail ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic