[PATCH v4 0/5] livepatch: Fail object initialization on duplicate patched function

[PATCH v4 0/5] livepatch: Fail object initialization on duplicate patched function

[Petr Mladek]

This is v4 of the patch failing object initialization when it patches
the same underlying function twice through two aliased symbols
(functions sharing one address) in the same object.

Changes against [v3]:

  + Add selftest for livepatching aliased symbols [Song, Harry]

  + Only partially revert relocations when failing inside
    klp_apply_object_relocs() [Sashiko]

  + Cleanup changes made by klp_init_object_loaded() for failing
    patch on another failures in klp_module_coming() [Sashiko]

Changes against [v2]:

  + Clarify comments and the commit message [Miroslav]

  + Cleanup when klp_init_object_loaded() fails [Sashiko]


[v2] https://lore.kernel.org/r/20260823060734.58443-1-x90613@gmail.com
[v3] https://lore.kernel.org/r/20260830173343.52759-1-x90613@gmail.com


Harry Hsu (2):
  livepatch: Fail object initialization on duplicate patched function
  selftests/livepatch: Test rejection of aliased symbols in one object

Petr Mladek (3):
  livepatch: Move code for updating livepatch object relocations
  livepatch: Clear relocations when klp_apply_object_relocs() fails
  livepatch: Clean up klp_init_object_loaded() when fails

 kernel/livepatch/core.c                       | 137 ++++++++++++------
 tools/testing/selftests/livepatch/Makefile    |   3 +-
 .../testing/selftests/livepatch/test-alias.sh |  81 +++++++++++
 .../selftests/livepatch/test_modules/Makefile |   4 +-
 .../test_modules/test_klp_alias_patch.c       |  62 ++++++++
 .../test_modules/test_klp_alias_target.c      |  48 ++++++
 6 files changed, 285 insertions(+), 50 deletions(-)
 create mode 100755 tools/testing/selftests/livepatch/test-alias.sh
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_alias_patch.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_alias_target.c

-- 
2.55.0

[PATCH v4 1/5] livepatch: Fail object initialization on duplicate patched function

[Petr Mladek]

From: Harry Hsu <x90613@gmail.com>

Several symbols can share one address:

  ffffffff8ed7fef0 t __do_sys_fork
  ffffffff8ed7fef0 T __ia32_sys_fork
  ffffffff8ed7fef0 T __x64_sys_fork

klp_find_ops() looks the ops up by func->old_func, i.e. by address, so
two klp_funcs of the same livepatch naming two of these symbols resolve
to the same klp_ops and are both pushed onto one ops->func_stack.

This breaks the assumption that a single livepatch contributes at most
one entry to any func_stack.  klp_ftrace_handler() picks the entry at
the top of the stack, but when both entries belong to the same livepatch
there is nothing that says which of them should be used in the PATCHED
state, and the UNPATCHED state has to end up at the original function
either way.  klp_check_stack_func() cannot tell them apart either: it
asks whether the preceding entry is the original function or another
livepatch's replacement, and an aliased sibling is neither.

Patching two aliases of one function from a single livepatch was never
meaningful, so fail object initialization in klp_init_object_loaded()
rather than leave the redirection undefined.

Fixes: 3c33f5b99d68 ("livepatch: support for repatching a function")
Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Harry Hsu <x90613@gmail.com>
---
 kernel/livepatch/core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a240d1144e89..a6762cbe74b7 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -863,7 +863,7 @@ static void klp_clear_object_relocs(struct klp_patch *patch,
 static int klp_init_object_loaded(struct klp_patch *patch,
 				  struct klp_object *obj)
 {
-	struct klp_func *func;
+	struct klp_func *func, *prev_func;
 	int ret;
 
 	if (klp_is_module(obj)) {
@@ -885,6 +885,21 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 		if (ret)
 			return ret;
 
+		/*
+		 * Aliased symbols share one address, so they would resolve to
+		 * the same klp_ops and stack up on a single ops->func_stack,
+		 * leaving the redirection ambiguous.
+		 */
+		klp_for_each_func(obj, prev_func) {
+			if (prev_func == func)
+				break;
+			if (prev_func->old_func == func->old_func) {
+				pr_err("'%s' and '%s' resolve to the same address, aliased symbols are not supported\n",
+				       prev_func->old_name, func->old_name);
+				return -EINVAL;
+			}
+		}
+
 		ret = kallsyms_lookup_size_offset((unsigned long)func->old_func,
 						  &func->old_size, NULL);
 		if (!ret) {
-- 
2.55.0

[PATCH v4 2/5] selftests/livepatch: Test rejection of aliased symbols in one object

[Petr Mladek]

From: Harry Hsu <x90613@gmail.com>

klp_init_object_loaded() now rejects an object whose klp_funcs resolve to
the same address, because aliased symbols would push two klp_funcs of one
livepatch onto a single ops->func_stack and leave the redirection
ambiguous.

Add a target module providing test_klp_alias_show() together with its
__alias() sibling, and a livepatch naming both of them.  Two test cases
cover both callers of klp_init_object_loaded(): the klp_enable_patch()
path, where the target module is loaded before the livepatch, and the
klp_module_coming() path, where the livepatch is loaded first and the
module loader has to refuse the target module.

Suggested-by: Song Liu <song@kernel.org>
Signed-off-by: Harry Hsu <x90613@gmail.com>
---
 tools/testing/selftests/livepatch/Makefile    |  3 +-
 .../testing/selftests/livepatch/test-alias.sh | 81 +++++++++++++++++++
 .../selftests/livepatch/test_modules/Makefile |  4 +-
 .../test_modules/test_klp_alias_patch.c       | 62 ++++++++++++++
 .../test_modules/test_klp_alias_target.c      | 48 +++++++++++
 5 files changed, 196 insertions(+), 2 deletions(-)
 create mode 100755 tools/testing/selftests/livepatch/test-alias.sh
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_alias_patch.c
 create mode 100644 tools/testing/selftests/livepatch/test_modules/test_klp_alias_target.c

diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
index a080eb54a215..ddbeff4cb53d 100644
--- a/tools/testing/selftests/livepatch/Makefile
+++ b/tools/testing/selftests/livepatch/Makefile
@@ -11,7 +11,8 @@ TEST_PROGS := \
 	test-ftrace.sh \
 	test-sysfs.sh \
 	test-syscall.sh \
-	test-kprobe.sh
+	test-kprobe.sh \
+	test-alias.sh
 
 TEST_FILES := settings
 
diff --git a/tools/testing/selftests/livepatch/test-alias.sh b/tools/testing/selftests/livepatch/test-alias.sh
new file mode 100755
index 000000000000..4ae701de0dbf
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test-alias.sh
@@ -0,0 +1,81 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2026 Harry Hsu <x90613@gmail.com>
+
+. $(dirname $0)/functions.sh
+
+MOD_TARGET=test_klp_alias_target
+MOD_LIVEPATCH=test_klp_alias_patch
+
+setup_config
+
+
+# $MOD_TARGET provides two symbols that share a single address.  A
+# livepatch naming both of them would push two klp_funcs of the same
+# patch onto one ops->func_stack, leaving the redirection ambiguous, so
+# klp_init_object_loaded() has to reject the object.
+#
+# - load the target module and verify it produces the original output
+# - verify that a livepatch naming both aliases fails to load
+# - verify that the target module has been left unpatched
+
+start_test "livepatch of two aliased symbols in one object"
+
+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_failing_mod $MOD_LIVEPATCH
+
+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: ${MOD_TARGET}_init
+% insmod test_modules/$MOD_LIVEPATCH.ko
+livepatch: 'test_klp_alias_show' and 'test_klp_alias_show_alias' resolve to the same address, aliased symbols are not supported
+insmod: ERROR: could not insert module test_modules/$MOD_LIVEPATCH.ko: Invalid parameters
+% rmmod $MOD_TARGET
+$MOD_TARGET: ${MOD_TARGET}_exit"
+
+
+# The same object is initialized from klp_module_coming() when the
+# livepatch is loaded while the target module is still absent.  There
+# the error has to be propagated to the module loader instead.
+#
+# - load the livepatch, it is accepted because the object is not loaded
+# - verify that loading the target module is refused afterwards
+
+start_test "aliased symbols in a module coming after the livepatch"
+
+load_lp $MOD_LIVEPATCH
+load_failing_mod $MOD_TARGET
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+check_result "% insmod test_modules/$MOD_LIVEPATCH.ko
+livepatch: enabling patch '$MOD_LIVEPATCH'
+livepatch: '$MOD_LIVEPATCH': initializing patching transition
+livepatch: '$MOD_LIVEPATCH': starting patching transition
+livepatch: '$MOD_LIVEPATCH': completing patching transition
+livepatch: '$MOD_LIVEPATCH': patching complete
+% insmod test_modules/$MOD_TARGET.ko
+livepatch: 'test_klp_alias_show' and 'test_klp_alias_show_alias' resolve to the same address, aliased symbols are not supported
+livepatch: failed to initialize patch '$MOD_LIVEPATCH' for module '$MOD_TARGET' (-22)
+livepatch: patch '$MOD_LIVEPATCH' failed for module '$MOD_TARGET', refusing to load module '$MOD_TARGET'
+insmod: ERROR: could not insert module test_modules/$MOD_TARGET.ko: Invalid parameters
+% echo 0 > $SYSFS_KLP_DIR/$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"
+
+exit 0
diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
index a13d398585dc..532403e2b5ff 100644
--- a/tools/testing/selftests/livepatch/test_modules/Makefile
+++ b/tools/testing/selftests/livepatch/test_modules/Makefile
@@ -1,7 +1,9 @@
 TESTMODS_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
 KDIR ?= /lib/modules/$(shell uname -r)/build
 
-obj-m += test_klp_atomic_replace.o \
+obj-m += test_klp_alias_patch.o \
+	test_klp_alias_target.o \
+	test_klp_atomic_replace.o \
 	test_klp_callbacks_busy.o \
 	test_klp_callbacks_demo.o \
 	test_klp_callbacks_demo2.o \
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_alias_patch.c b/tools/testing/selftests/livepatch/test_modules/test_klp_alias_patch.c
new file mode 100644
index 000000000000..1b50088bc92d
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_alias_patch.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Harry Hsu <x90613@gmail.com>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include <linux/seq_file.h>
+
+static int livepatch_alias_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%s: %s\n", THIS_MODULE->name,
+		   "this has been live patched");
+	return 0;
+}
+
+/*
+ * Both names resolve to one address, so they end up on a single
+ * ops->func_stack and the redirection would be ambiguous.  Loading this
+ * livepatch is expected to fail.
+ */
+static struct klp_func funcs[] = {
+	{
+		.old_name = "test_klp_alias_show",
+		.new_func = livepatch_alias_show,
+	},
+	{
+		.old_name = "test_klp_alias_show_alias",
+		.new_func = livepatch_alias_show,
+	},
+	{},
+};
+
+static struct klp_object objs[] = {
+	{
+		.name = "test_klp_alias_target",
+		.funcs = funcs,
+	},
+	{},
+};
+
+static struct klp_patch patch = {
+	.mod = THIS_MODULE,
+	.objs = objs,
+};
+
+static int test_klp_alias_patch_init(void)
+{
+	return klp_enable_patch(&patch);
+}
+
+static void test_klp_alias_patch_exit(void)
+{
+}
+
+module_init(test_klp_alias_patch_init);
+module_exit(test_klp_alias_patch_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_AUTHOR("Harry Hsu <x90613@gmail.com>");
+MODULE_DESCRIPTION("Livepatch test: patch two aliased symbols of one object");
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_alias_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_alias_target.c
new file mode 100644
index 000000000000..b0f5fc35adf8
--- /dev/null
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_alias_target.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Harry Hsu <x90613@gmail.com>
+
+#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_alias_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
+	return 0;
+}
+
+/*
+ * Alias the function above so that both names resolve to one address, the
+ * way __do_sys_fork(), __ia32_sys_fork() and __x64_sys_fork() do in vmlinux.
+ * Nothing calls the alias, it only has to show up in the module's symbol
+ * table for the livepatch to name it.
+ */
+static int test_klp_alias_show_alias(struct seq_file *m, void *v)
+	__used __alias(test_klp_alias_show);
+
+static int test_klp_alias_target_init(void)
+{
+	pr_info("%s\n", __func__);
+	pde = proc_create_single("test_klp_alias_target", 0, NULL,
+				 test_klp_alias_show);
+	if (!pde)
+		return -ENOMEM;
+	return 0;
+}
+
+static void test_klp_alias_target_exit(void)
+{
+	pr_info("%s\n", __func__);
+	proc_remove(pde);
+}
+
+module_init(test_klp_alias_target_init);
+module_exit(test_klp_alias_target_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Harry Hsu <x90613@gmail.com>");
+MODULE_DESCRIPTION("Livepatch test: target module with two aliased symbols");
-- 
2.55.0

[PATCH v4 3/5] livepatch: Move code for updating livepatch object relocations

[Petr Mladek]

klp_free_object_loaded() is supposed to clear changes made by
klp_init_object_loaded(). It should call klp_clear_object_relocs()
which is currently defined later.

Move the code for updating object relocations up.

This is just a preparation step. No functional changes.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c | 72 ++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a6762cbe74b7..a6796cd6b65f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -342,6 +342,42 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
 					secndx, objname, true);
 }
 
+static int klp_write_object_relocs(struct klp_patch *patch,
+				   struct klp_object *obj,
+				   bool apply)
+{
+	int i, ret;
+	struct klp_modinfo *info = patch->mod->klp_info;
+
+	for (i = 1; i < info->hdr.e_shnum; i++) {
+		Elf_Shdr *sec = info->sechdrs + i;
+
+		if (!(sec->sh_flags & SHF_RELA_LIVEPATCH))
+			continue;
+
+		ret = klp_write_section_relocs(patch->mod, info->sechdrs,
+					       info->secstrings,
+					       patch->mod->core_kallsyms.strtab,
+					       info->symndx, i, obj->name, apply);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int klp_apply_object_relocs(struct klp_patch *patch,
+				   struct klp_object *obj)
+{
+	return klp_write_object_relocs(patch, obj, true);
+}
+
+static void klp_clear_object_relocs(struct klp_patch *patch,
+				    struct klp_object *obj)
+{
+	klp_write_object_relocs(patch, obj, false);
+}
+
 /*
  * Sysfs Interface
  *
@@ -823,42 +859,6 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
 			   func->old_sympos ? func->old_sympos : 1);
 }
 
-static int klp_write_object_relocs(struct klp_patch *patch,
-				   struct klp_object *obj,
-				   bool apply)
-{
-	int i, ret;
-	struct klp_modinfo *info = patch->mod->klp_info;
-
-	for (i = 1; i < info->hdr.e_shnum; i++) {
-		Elf_Shdr *sec = info->sechdrs + i;
-
-		if (!(sec->sh_flags & SHF_RELA_LIVEPATCH))
-			continue;
-
-		ret = klp_write_section_relocs(patch->mod, info->sechdrs,
-					       info->secstrings,
-					       patch->mod->core_kallsyms.strtab,
-					       info->symndx, i, obj->name, apply);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
-static int klp_apply_object_relocs(struct klp_patch *patch,
-				   struct klp_object *obj)
-{
-	return klp_write_object_relocs(patch, obj, true);
-}
-
-static void klp_clear_object_relocs(struct klp_patch *patch,
-				    struct klp_object *obj)
-{
-	klp_write_object_relocs(patch, obj, false);
-}
-
 /* parts of the initialization that is done only when the object is loaded */
 static int klp_init_object_loaded(struct klp_patch *patch,
 				  struct klp_object *obj)
-- 
2.55.0

[PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails

[Petr Mladek]

When a module is loaded, klp_module_coming() updates all enabled
livepatches. If an error occurs, it delegates cleanup to
klp_cleanup_module_patches_limited(). However, this cleanup loop skips
the partially updated patch, leaving any changes made prior to failure
unreverted.

One unhandled failure path occurs inside klp_apply_object_relocs(). On
architectures like x86_64, apply_relocate_add() performs a verification
step using memcmp() to check that memory contains the expected relocated
or zeroed value. If relocations left behind by a failed patch are not
cleared, subsequent patch operations or reloads can fail this validation.

Introduce klp_write_object_relocs_limited() to unwind and clear only the
relocations that were successfully applied before klp_write_object_relocs()
encountered an error.

There is no need to clear relocations for other objects in the failing
patch because klp_module_coming() operates strictly on the specific
module being loaded.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a6796cd6b65f..714f97fdd271 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -342,14 +342,17 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
 					secndx, objname, true);
 }
 
-static int klp_write_object_relocs(struct klp_patch *patch,
-				   struct klp_object *obj,
-				   bool apply)
+static int klp_write_object_relocs_limited(struct klp_patch *patch,
+					   struct klp_object *obj,
+					   bool apply, int limit)
 {
 	int i, ret;
 	struct klp_modinfo *info = patch->mod->klp_info;
 
-	for (i = 1; i < info->hdr.e_shnum; i++) {
+	if (!limit || limit > info->hdr.e_shnum)
+		limit = info->hdr.e_shnum;
+
+	for (i = 1; i < limit; i++) {
 		Elf_Shdr *sec = info->sechdrs + i;
 
 		if (!(sec->sh_flags & SHF_RELA_LIVEPATCH))
@@ -359,13 +362,23 @@ static int klp_write_object_relocs(struct klp_patch *patch,
 					       info->secstrings,
 					       patch->mod->core_kallsyms.strtab,
 					       info->symndx, i, obj->name, apply);
-		if (ret)
+		if (ret) {
+			if (apply)
+				klp_write_object_relocs_limited(patch, obj, false, i);
 			return ret;
+		}
 	}
 
 	return 0;
 }
 
+static int klp_write_object_relocs(struct klp_patch *patch,
+				   struct klp_object *obj,
+				   bool apply)
+{
+	return klp_write_object_relocs_limited(patch, obj, apply, 0);
+}
+
 static int klp_apply_object_relocs(struct klp_patch *patch,
 				   struct klp_object *obj)
 {
-- 
2.55.0

[PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails

[Petr Mladek]

When loading a module, klp_module_coming() updates all enabled patches.
If an error occurs, klp_cleanup_module_patches_limited() cleans up fully
processed patches, but skips the patch that failed midway.

The current code is a bit messy. The changes done by
klp_init_object_loaded() should get cleared by klp_free_object_loaded().
But this function also clears obj->mod which is set by
klp_module_coming(). And relocations are cleared separately.

Fix the situations by updating klp_free_object_loaded(). It should
revert all and only changes made by klp_init_object_loaded().
This requires some shuffling:

 + Clear obj->mod explicitly in klp_cleanup_module_patches_limited()
   and do not rely on klp_free_object_loaded().

 + Clear relocations in klp_free_object_loaded(). Remove the explicit
   call from klp_cleanup_module_patches_limited(). This requires
   adding the @patch parameter.

Next, klp_init_object_loaded() has to clear its own changes on
failure. It just returns an error when relocations failed because
they clear their own mess. It could call klp_free_object_loaded()
in other situations because all relocations were done and other
values are just cleared.

Finally, in klp_module_coming(), avoid code duplication by goto targets.
There is no need to clear relocations for other objects in the failing
patch because klp_module_coming() operates strictly on the specific
module being loaded.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@smtp.kernel.org
Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 714f97fdd271..87b2331486d4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -738,18 +738,20 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only)
 }
 
 /* Clean up when a patched object is unloaded */
-static void klp_free_object_loaded(struct klp_object *obj)
+static void klp_free_object_loaded(struct klp_patch *patch,
+				   struct klp_object *obj)
 {
 	struct klp_func *func;
 
-	obj->mod = NULL;
-
 	klp_for_each_func(obj, func) {
 		func->old_func = NULL;
 
 		if (func->nop)
 			func->new_func = NULL;
 	}
+
+	if (klp_is_module(obj))
+		klp_clear_object_relocs(patch, obj);
 }
 
 static void __klp_free_objects(struct klp_patch *patch, bool nops_only)
@@ -896,7 +898,7 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 					     func->old_sympos,
 					     (unsigned long *)&func->old_func);
 		if (ret)
-			return ret;
+			goto err;
 
 		/*
 		 * Aliased symbols share one address, so they would resolve to
@@ -909,7 +911,8 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 			if (prev_func->old_func == func->old_func) {
 				pr_err("'%s' and '%s' resolve to the same address, aliased symbols are not supported\n",
 				       prev_func->old_name, func->old_name);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err;
 			}
 		}
 
@@ -918,7 +921,8 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 		if (!ret) {
 			pr_err("kallsyms size lookup failed for '%s'\n",
 			       func->old_name);
-			return -ENOENT;
+			ret = -ENOENT;
+			goto err;
 		}
 
 		if (func->nop)
@@ -929,11 +933,17 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 		if (!ret) {
 			pr_err("kallsyms size lookup failed for '%s' replacement\n",
 			       func->old_name);
-			return -ENOENT;
+			ret = -ENOENT;
+			goto err;
 		}
 	}
 
 	return 0;
+
+err:
+	klp_free_object_loaded(patch, obj);
+
+	return ret;
 }
 
 static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
@@ -1287,8 +1297,8 @@ static void klp_cleanup_module_patches_limited(struct module *mod,
 			klp_unpatch_object(obj);
 
 			klp_post_unpatch_callback(obj);
-			klp_clear_object_relocs(patch, obj);
-			klp_free_object_loaded(obj);
+			klp_free_object_loaded(patch, obj);
+			obj->mod = NULL;
 			break;
 		}
 	}
@@ -1337,7 +1347,7 @@ int klp_module_coming(struct module *mod)
 			if (ret) {
 				pr_warn("pre-patch callback failed for object '%s'\n",
 					obj->name);
-				goto err;
+				goto err_free_object;
 			}
 
 			ret = klp_patch_object(obj);
@@ -1345,8 +1355,7 @@ int klp_module_coming(struct module *mod)
 				pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
 					patch->mod->name, obj->mod->name, ret);
 
-				klp_post_unpatch_callback(obj);
-				goto err;
+				goto err_unpatch_callback;
 			}
 
 			if (patch != klp_transition_patch)
@@ -1360,6 +1369,10 @@ int klp_module_coming(struct module *mod)
 
 	return 0;
 
+err_unpatch_callback:
+	klp_post_unpatch_callback(obj);
+err_free_object:
+	klp_free_object_loaded(patch, obj);
 err:
 	/*
 	 * If a patch is unsuccessfully applied, return
-- 
2.55.0

Re: [PATCH v4 1/5] livepatch: Fail object initialization on duplicate patched function

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Atomic replace patches will fail to load if previous patches targeted aliased functions.
--

commit e5553650270f93e11fe885d264a3829802223d3a
Author: Harry Hsu <x90613@gmail.com>

livepatch: Fail object initialization on duplicate patched function

This commit introduces a check to prevent patching multiple aliases of a
function within a single livepatch. It fails object initialization if aliases
resolving to the same address are detected.

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index a240d1144e89c..a6762cbe74b70 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
[ ... ]
> @@ -885,6 +885,21 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>  		if (ret)
>  			return ret;
>  
> +		/*
> +		 * Aliased symbols share one address, so they would resolve to
> +		 * the same klp_ops and stack up on a single ops->func_stack,
> +		 * leaving the redirection ambiguous.
> +		 */
> +		klp_for_each_func(obj, prev_func) {
> +			if (prev_func == func)
> +				break;
> +			if (prev_func->old_func == func->old_func) {
> +				pr_err("'%s' and '%s' resolve to the same address, aliased symbols are not supported\n",
> +				       prev_func->old_name, func->old_name);
> +				return -EINVAL;

[Severity: High]
Will this check inadvertently reject nop functions generated by an atomic
replace patch when previous patches targeted aliased functions?

If previously loaded livepatches targeted aliased functions (for example, one
patch targets __do_sys_fork and another targets __x64_sys_fork), they were
permitted since they resided in different patches.

When an atomic replace patch is loaded, klp_add_nops() loops over all
previously patched functions and adds nops for them to the new patch. Since
nop deduplication is based on old_name, nops for both aliases are added.

Later, this check in klp_init_object_loaded() resolves their addresses and
finds duplicate old_func addresses. This causes the initialization to fail
with -EINVAL, which prevents the system from atomically replacing existing
livepatches and might force a reboot if the old patches cannot be safely
disabled manually.

> +			}
> +		}
> +
>  		ret = kallsyms_lookup_size_offset((unsigned long)func->old_func,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908120325.299649-1-pmladek@suse.com?part=1

Re: [PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Off-by-one error in the rollback loop leaves the failing section's partially applied relocations uncleared.
--

commit d297505bb4b1d833ec93725770d65ac1044abcad
Author: Petr Mladek <pmladek@suse.com>

livepatch: Clear relocations when klp_apply_object_relocs() fails

This commit introduces klp_write_object_relocs_limited() to unwind and
clear only the relocations that were successfully applied before
klp_write_object_relocs() encountered an error. This prevents partially
applied patches from leaving un-reverted changes that cause subsequent
patch operations to fail validation.

Link: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index a6796cd6b65fd..714f97fdd271a 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -342,14 +342,17 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
[ ... ]
> +static int klp_write_object_relocs_limited(struct klp_patch *patch,
> +					   struct klp_object *obj,
> +					   bool apply, int limit)
>  {
>  	int i, ret;
>  	struct klp_modinfo *info = patch->mod->klp_info;
>  
> -	for (i = 1; i < info->hdr.e_shnum; i++) {
> +	if (!limit || limit > info->hdr.e_shnum)
> +		limit = info->hdr.e_shnum;
> +
> +	for (i = 1; i < limit; i++) {
>  		Elf_Shdr *sec = info->sechdrs + i;
[ ... ]
> -		if (ret)
> +		if (ret) {
> +			if (apply)
> +				klp_write_object_relocs_limited(patch, obj, false, i);

[Severity: Medium]
When an error occurs in klp_write_section_relocs(), the rollback is triggered
with the failing section index passed as the limit. Since the rollback loop
iterates with a strict less-than bounds check against this limit, will this
skip the rollback for any relocations that were successfully applied within
the failing section itself before it encountered the error?

It looks like this could leave partially applied relocations permanently
in memory within the failing section, causing future module loads to fail
validation.

>  			return ret;
> +		}
>  	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908120325.299649-1-pmladek@suse.com?part=4

Re: [PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Partial relocations are not reverted when apply_relocate_add() fails midway, leading to permanent state corruption.
--

commit 3f6df1b4ed699e839abbdc8919a6c7de5d1850c8
Author: Petr Mladek <pmladek@suse.com>

livepatch: Clean up klp_init_object_loaded() when fails

When loading a module, klp_module_coming() updates all enabled patches.
If an error occurs, klp_cleanup_module_patches_limited() cleans up fully
processed patches, but skips the patch that failed midway. This commit
updates klp_free_object_loaded() to revert all and only changes made by
klp_init_object_loaded().

Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@smtp.kernel.org
Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org

> Next, klp_init_object_loaded() has to clear its own changes on
> failure. It just returns an error when relocations failed because
> they clear their own mess. It could call klp_free_object_loaded()
> in other situations because all relocations were done and other
> values are just cleared.

[Severity: High]
Does this actually clear its own mess completely? Looking at how relocations
are rolled back in klp_write_object_relocs_limited(), which was introduced
earlier in this series:

kernel/livepatch/core.c:klp_write_object_relocs_limited() {
    ...
		ret = klp_write_section_relocs(patch->mod, info->sechdrs,
					       info->secstrings,
					       patch->mod->core_kallsyms.strtab,
					       info->symndx, i, obj->name, apply);
		if (ret) {
			if (apply)
				klp_write_object_relocs_limited(patch, obj, false, i);
			return ret;
		}
    ...
}

By passing i as the limit to the rollback function, the rollback loop:

	for (i = 1; i < limit; i++) {

will terminate at i - 1. Doesn't this skip the rollback for the partially
applied section i?

If apply_relocate_add succeeds on relocations 0 to k-1 in section i, but
fails on relocation k, these partial relocations remain permanently applied
in the livepatch module's text. 

Subsequent attempts to load the target module will fail immediately at
relocation 0 due to non-zero target memory, converting a potentially transient
address-space layout failure into a permanent -ENOEXEC on retry.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908120325.299649-1-pmladek@suse.com?part=5

Re: [PATCH v4 1/5] livepatch: Fail object initialization on duplicate patched function

[Petr Mladek]

On Tue 2026-09-08 12:17:44, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Atomic replace patches will fail to load if previous patches targeted aliased functions.
> --
> 
> commit e5553650270f93e11fe885d264a3829802223d3a
> Author: Harry Hsu <x90613@gmail.com>
> 
> livepatch: Fail object initialization on duplicate patched function
> 
> This commit introduces a check to prevent patching multiple aliases of a
> function within a single livepatch. It fails object initialization if aliases
> resolving to the same address are detected.
> 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index a240d1144e89c..a6762cbe74b70 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> [ ... ]
> > @@ -885,6 +885,21 @@ static int klp_init_object_loaded(struct klp_patch *patch,
> >  		if (ret)
> >  			return ret;
> >  
> > +		/*
> > +		 * Aliased symbols share one address, so they would resolve to
> > +		 * the same klp_ops and stack up on a single ops->func_stack,
> > +		 * leaving the redirection ambiguous.
> > +		 */
> > +		klp_for_each_func(obj, prev_func) {
> > +			if (prev_func == func)
> > +				break;
> > +			if (prev_func->old_func == func->old_func) {
> > +				pr_err("'%s' and '%s' resolve to the same address, aliased symbols are not supported\n",
> > +				       prev_func->old_name, func->old_name);
> > +				return -EINVAL;
> 
> [Severity: High]
> Will this check inadvertently reject nop functions generated by an atomic
> replace patch when previous patches targeted aliased functions?
> 
> If previously loaded livepatches targeted aliased functions (for example, one
> patch targets __do_sys_fork and another targets __x64_sys_fork), they were
> permitted since they resided in different patches.
> 
> When an atomic replace patch is loaded, klp_add_nops() loops over all
> previously patched functions and adds nops for them to the new patch. Since
> nop deduplication is based on old_name, nops for both aliases are added.
> 
> Later, this check in klp_init_object_loaded() resolves their addresses and
> finds duplicate old_func addresses. This causes the initialization to fail
> with -EINVAL, which prevents the system from atomically replacing existing
> livepatches and might force a reboot if the old patches cannot be safely
> disabled manually.

Wow, this is a nice catch. I would say that it is a corner case but
the problem seems to exist. Now, the question is how to deal with it.

I see three possibilities:

1. We might detect the aliases in klp_find_func() and create only
   one "nop" entry or do not create it all.

2. The problem might be easier to solve after adding
   the provides/obsoletes, aka replace set, feature [1].
   It should prevent loading the other livepatch for the aliased
   symbol in the 1st place.

3. Just document the problem as a limitation. It might be hit only
   when installing more liveapatches in parallel that current
   wild way.

I personally prefer to wait for the provides/requires feature.

[1] https://lore.kernel.org/all/20260607131659.29281-1-laoar.shao@gmail.com/

Best Regards,
Petr

> > +			}
> > +		}
> > +
> >  		ret = kallsyms_lookup_size_offset((unsigned long)func->old_func,
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908120325.299649-1-pmladek@suse.com?part=1

Re: [PATCH v4 4/5] livepatch: Clear relocations when klp_apply_object_relocs() fails

[Petr Mladek]

On Tue 2026-09-08 12:18:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Off-by-one error in the rollback loop leaves the failing section's partially applied relocations uncleared.
> --
> 
> commit d297505bb4b1d833ec93725770d65ac1044abcad
> Author: Petr Mladek <pmladek@suse.com>
> 
> livepatch: Clear relocations when klp_apply_object_relocs() fails
> 
> This commit introduces klp_write_object_relocs_limited() to unwind and
> clear only the relocations that were successfully applied before
> klp_write_object_relocs() encountered an error. This prevents partially
> applied patches from leaving un-reverted changes that cause subsequent
> patch operations to fail validation.
> 
> Link: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org
> 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index a6796cd6b65fd..714f97fdd271a 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -342,14 +342,17 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,
> [ ... ]
> > +static int klp_write_object_relocs_limited(struct klp_patch *patch,
> > +					   struct klp_object *obj,
> > +					   bool apply, int limit)
> >  {
> >  	int i, ret;
> >  	struct klp_modinfo *info = patch->mod->klp_info;
> >  
> > -	for (i = 1; i < info->hdr.e_shnum; i++) {
> > +	if (!limit || limit > info->hdr.e_shnum)
> > +		limit = info->hdr.e_shnum;
> > +
> > +	for (i = 1; i < limit; i++) {
> >  		Elf_Shdr *sec = info->sechdrs + i;
> [ ... ]
> > -		if (ret)
> > +		if (ret) {
> > +			if (apply)
> > +				klp_write_object_relocs_limited(patch, obj, false, i);
> 
> [Severity: Medium]
> When an error occurs in klp_write_section_relocs(), the rollback is triggered
> with the failing section index passed as the limit. Since the rollback loop
> iterates with a strict less-than bounds check against this limit, will this
> skip the rollback for any relocations that were successfully applied within
> the failing section itself before it encountered the error?
> 
> It looks like this could leave partially applied relocations permanently
> in memory within the failing section, causing future module loads to fail
> validation.

Sigh, I did not go deep enough. And it goes down to the arch-specific
implementations, in particular arch/x86/kernel/module.c:

static int __write_relocate_add(Elf64_Shdr *sechdrs,
		   const char *strtab,
		   unsigned int symindex,
		   unsigned int relsec,
		   struct module *me,
		   void *(*write)(void *dest, const void *src, size_t len),
		   bool apply)
{
[...]
	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
[...]
			if (memcmp(loc, &zero, size)) {
				pr_err("x86/modules: Invalid relocation target, existing value is nonzero for sec %u, idx %u, type %d, loc %lx, val %llx\n",
				       relsec, i, (int)ELF64_R_TYPE(rel[i].r_info),
				       (unsigned long)loc, val);
				return -ENOEXEC;
			}
			write(loc, &val, size);
		} else {
			if (memcmp(loc, &val, size)) {
				pr_warn("x86/modules: Invalid relocation target, existing value does not match expected value for sec %u, idx %u, type %d, loc %lx, val %llx\n",
					relsec, i, (int)ELF64_R_TYPE(rel[i].r_info),
					(unsigned long)loc, val);
				return -ENOEXEC;
			}
			write(loc, &zero, size);
		}
[...]

We would need to implement the revert at this level.
IMHO, it could be done separately.

I used Gemini LLM to check it and it seems that x86_64 is the only
architecture with these permissive checks. It seems that all other
architectures just write the new value without checking
the existing one.

Would you go this way and try to get patch into x86 code?
Should I do it in v5 or separately?

Best Regards,
Petr

PS: I would prefer to go with unless it causes regression
    and fix this "later". We could not endlessly delay
    fixes because of Sashiko finding pre-existing problems.

> >  			return ret;
> > +		}
> >  	}
> >  
> >  	return 0;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908120325.299649-1-pmladek@suse.com?part=4

Re: [PATCH v4 5/5] livepatch: Clean up klp_init_object_loaded() when fails

[Petr Mladek]

On Tue 2026-09-08 12:25:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Partial relocations are not reverted when apply_relocate_add() fails midway, leading to permanent state corruption.
> --
> 
> commit 3f6df1b4ed699e839abbdc8919a6c7de5d1850c8
> Author: Petr Mladek <pmladek@suse.com>
> 
> livepatch: Clean up klp_init_object_loaded() when fails
> 
> When loading a module, klp_module_coming() updates all enabled patches.
> If an error occurs, klp_cleanup_module_patches_limited() cleans up fully
> processed patches, but skips the patch that failed midway. This commit
> updates klp_free_object_loaded() to revert all and only changes made by
> klp_init_object_loaded().
> 
> Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@smtp.kernel.org
> Closes: https://lore.kernel.org/r/20260830175608.4BABB1F000E9@smtp.kernel.org
> 
> > Next, klp_init_object_loaded() has to clear its own changes on
> > failure. It just returns an error when relocations failed because
> > they clear their own mess. It could call klp_free_object_loaded()
> > in other situations because all relocations were done and other
> > values are just cleared.
> 
> [Severity: High]
> Does this actually clear its own mess completely? Looking at how relocations
> are rolled back in klp_write_object_relocs_limited(), which was introduced
> earlier in this series:
> 
> kernel/livepatch/core.c:klp_write_object_relocs_limited() {
>     ...
> 		ret = klp_write_section_relocs(patch->mod, info->sechdrs,
> 					       info->secstrings,
> 					       patch->mod->core_kallsyms.strtab,
> 					       info->symndx, i, obj->name, apply);
> 		if (ret) {
> 			if (apply)
> 				klp_write_object_relocs_limited(patch, obj, false, i);
> 			return ret;
> 		}
>     ...
> }
> 
> By passing i as the limit to the rollback function, the rollback loop:
> 
> 	for (i = 1; i < limit; i++) {
> 
> will terminate at i - 1. Doesn't this skip the rollback for the partially
> applied section i?
> 
> If apply_relocate_add succeeds on relocations 0 to k-1 in section i, but
> fails on relocation k, these partial relocations remain permanently applied
> in the livepatch module's text. 

This is the same problem which was reported for the 4th patch.
It would need to handled in the arch-code, namely
in __write_relocate_add() in arch/x86/kernel/module.c.

Best Regards,
Petr

> Subsequent attempts to load the target module will fail immediately at
> relocation 0 due to non-zero target memory, converting a potentially transient
> address-space layout failure into a permanent -ENOEXEC on retry.