[PATCH v3 0/3] livepatch: Fail object initialization on duplicate patched function

[PATCH v3 0/3] livepatch: Fail object initialization on duplicate patched function

[Harry Hsu]

This is v3 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 [1].

Per Miroslav's review, the changelog and title have been reworded to
avoid implying this is livepatch-specific validation: the check runs
during object initialization, which klp_module_coming() also triggers
for a module still being loaded.

Petr found that the error path in klp_init_object_loaded() -- taken
by this new check, among others -- leaves stale object state behind
on failure. His two patches (2/3, 3/3) fix that by routing all of
klp_init_object_loaded()'s error returns through proper cleanup, and
are included in this series since they touch the same code and would
otherwise conflict with patch 1/3.

[1] https://lore.kernel.org/all/20260823060734.58443-1-x90613@gmail.com/

Harry Hsu (1):
  livepatch: Fail object initialization on duplicate patched function

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

 kernel/livepatch/core.c | 117 ++++++++++++++++++++++++----------------
 1 file changed, 71 insertions(+), 46 deletions(-)

-- 
2.43.0

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

[Harry Hsu]

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 28d15ba58a26..0dd8cda5c9b8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -866,7 +866,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)) {
@@ -888,6 +888,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.43.0

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

[Harry Hsu]

From: Petr Mladek <pmladek@suse.com>

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 0dd8cda5c9b8..aa895e8259b0 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
  *
@@ -826,42 +862,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.43.0

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

[Harry Hsu]

From: Petr Mladek <pmladek@suse.com>

When a module is loaded, klp_module_coming() iterates over patches and
calls klp_init_object_loaded(). If initialization fails, it delegates
cleanup to klp_cleanup_module_patches_limited().

However, the cleanup loop skips the failing patch. Each function called
in klp_init_object_loaded() is supposed to clean its own changes. This
works except for the changes done by klp_init_object_loaded().

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.

Finally, call klp_free_object_loaded() in the error path in
klp_init_object_loaded().

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260823062313.1321B1F000E9@smtp.kernel.org
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index aa895e8259b0..4fff00a40d88 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -725,18 +725,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)
@@ -878,7 +880,7 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 		 */
 		ret = klp_apply_object_relocs(patch, obj);
 		if (ret)
-			return ret;
+			goto err;
 	}
 
 	klp_for_each_func(obj, func) {
@@ -886,7 +888,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
@@ -899,7 +901,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;
 			}
 		}
 
@@ -908,7 +911,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)
@@ -919,11 +923,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)
@@ -1264,8 +1274,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;
 		}
 	}
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Failing `klp_apply_object_relocs()` triggers spurious architectural warnings due to unconditional clearing of unapplied relocations in the newly added error path.

Pre-existing issues:
- [High] If `klp_pre_patch_callback()` or `klp_patch_object()` fails during `klp_module_coming()`, the successfully initialized state from `klp_init_object_loaded()` is leaked. The cleanup function `klp_cleanup_module_patches_limited()` intentionally skips the currently failing patch, leaving applied relocations in the livepatch module uncleared and causing subsequent load attempts of the target module to persistently fail.
--

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

livepatch: Clean up klp_init_object_loaded() when fails

When a module is loaded, klp_module_coming() iterates over patches and
calls klp_init_object_loaded(). If initialization fails, it delegates
cleanup to klp_cleanup_module_patches_limited(). 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

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index a6796cd6b65fd..706a4a7193514 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c

[ ... ]

> @@ -916,11 +920,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;
>  }

[Severity: Medium]
Does this unconditionally clear unapplied relocations?

When klp_init_object_loaded() fails (for example, if a symbol lookup fails
after klp_apply_object_relocs() succeeds, or if klp_apply_object_relocs()
itself fails), this newly added error path calls klp_free_object_loaded().

This unconditionally invokes klp_clear_object_relocs(), which iterates over
all .klp.rela sections and attempts to un-apply them via
clear_relocate_add().

However, for sections that were never applied, the target memory remains
unmodified (zeroed). On architectures like x86, clear_relocate_add() uses
memcmp() to verify that the memory contains the expected relocated value.
Since it does not, it prints an "Invalid relocation target" warning for
every unapplied relocation entry, resulting in significant spurious warning
spam in the kernel log.

[ ... ]

> @@ -1274,8 +1284,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;
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but does this cleanup logic leak relocations
if klp_module_coming() fails after klp_init_object_loaded() succeeds?

If klp_pre_patch_callback() or klp_patch_object() fails during
klp_module_coming(), the successfully initialized state from
klp_init_object_loaded() is leaked.

The cleanup function intentionally skips the currently failing patch:

kernel/livepatch/core.c:klp_cleanup_module_patches_limited() {
	...
	klp_for_each_patch(patch) {
		if (patch == limit)
			break;
	...
}

This prevents its newly applied relocations from being cleared. Since these
relocations modify the livepatch module's memory, they persist even after
the target module's load attempt is aborted.

Upon a subsequent attempt to load the target module,
klp_apply_object_relocs() will attempt to apply the relocations again. On
architectures like x86, apply_relocate_add() verifies that the target
memory is zero; finding it non-zero, it returns -ENOEXEC. This causes
klp_module_coming() to fail, permanently preventing the target module
from being loaded as long as the livepatch is loaded.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830173343.52759-1-x90613@gmail.com?part=3

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

[Petr Mladek]

On Sun 2026-08-30 17:56:07, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Failing `klp_apply_object_relocs()` triggers spurious architectural warnings due to unconditional clearing of unapplied relocations in the newly added error path.
> 
> Pre-existing issues:
> - [High] If `klp_pre_patch_callback()` or `klp_patch_object()` fails during `klp_module_coming()`, the successfully initialized state from `klp_init_object_loaded()` is leaked. The cleanup function `klp_cleanup_module_patches_limited()` intentionally skips the currently failing patch, leaving applied relocations in the livepatch module uncleared and causing subsequent load attempts of the target module to persistently fail.
> --
> 
> commit a90c142b7a6f2c5551f83382ec1fcf15614ffbac
> Author: Petr Mladek <pmladek@suse.com>
> 
> livepatch: Clean up klp_init_object_loaded() when fails
> 
> When a module is loaded, klp_module_coming() iterates over patches and
> calls klp_init_object_loaded(). If initialization fails, it delegates
> cleanup to klp_cleanup_module_patches_limited(). 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
> 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index a6796cd6b65fd..706a4a7193514 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> 
> [ ... ]
> 
> > @@ -916,11 +920,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;
> >  }
> 
> [Severity: Medium]
> Does this unconditionally clear unapplied relocations?

Ah, I thought that this should be OK.

> When klp_init_object_loaded() fails (for example, if a symbol lookup fails
> after klp_apply_object_relocs() succeeds, or if klp_apply_object_relocs()
> itself fails), this newly added error path calls klp_free_object_loaded().
> 
> This unconditionally invokes klp_clear_object_relocs(), which iterates over
> all .klp.rela sections and attempts to un-apply them via
> clear_relocate_add().
> 
> However, for sections that were never applied, the target memory remains
> unmodified (zeroed). On architectures like x86, clear_relocate_add() uses
> memcmp() to verify that the memory contains the expected relocated value.
> Since it does not, it prints an "Invalid relocation target" warning for
> every unapplied relocation entry, resulting in significant spurious warning
> spam in the kernel log.

Sashiko is right. __write_relocate_add() really checks the original
value even when clearing relocation at least on x86_64.

Only klp_write_object_relocs() knows which relocations were updated.
So it has to clear its own mess.

I am going to do the following changes on top of this patch
(just compile tested at the moment):

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 1c217ac49d2b..b6ae3a518ca9 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)
 {
@@ -902,7 +915,7 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 		 */
 		ret = klp_apply_object_relocs(patch, obj);
 		if (ret)
-			goto err;
+			return ret;
 	}
 
 	klp_for_each_func(obj, func) {



> [ ... ]
> 
> > @@ -1274,8 +1284,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;
> >  		}
> >  	}
> 
> [Severity: High]
> This is a pre-existing issue, but does this cleanup logic leak relocations
> if klp_module_coming() fails after klp_init_object_loaded() succeeds?
> 
> If klp_pre_patch_callback() or klp_patch_object() fails during
> klp_module_coming(), the successfully initialized state from
> klp_init_object_loaded() is leaked.
> 
> The cleanup function intentionally skips the currently failing patch:
> 
> kernel/livepatch/core.c:klp_cleanup_module_patches_limited() {
> 	...
> 	klp_for_each_patch(patch) {
> 		if (patch == limit)
> 			break;
> 	...
> }
> 
> This prevents its newly applied relocations from being cleared. Since these
> relocations modify the livepatch module's memory, they persist even after
> the target module's load attempt is aborted.
> 
> Upon a subsequent attempt to load the target module,
> klp_apply_object_relocs() will attempt to apply the relocations again. On
> architectures like x86, apply_relocate_add() verifies that the target
> memory is zero; finding it non-zero, it returns -ENOEXEC. This causes
> klp_module_coming() to fail, permanently preventing the target module
> from being loaded as long as the livepatch is loaded.

Sigh, I got a bit lost in all the cycles. I believe that this should
get fixed the the following changes on top of this patch:

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 1c217ac49d2b..09b6f4aa6217 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1360,7 +1373,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);
@@ -1368,8 +1381,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)
@@ -1383,6 +1395,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


Note: This is called when it fails in the middle of

		klp_for_each_object(patch, obj) {

      Naive approach would be to implement another *_limited
      variant which would revert the action for all already
      proceed "obj" structures.

      But this is called in klp_module_coming() so only one
      struct object should match. All others are skipped.
      This is why it should be enough to revert only the last "obj"
      in the err_* goto targets.

      We propably should enforce this => add another patch
      which would reject livepatches which contain two
      struct object for the same object.

Best Regards,
Petr

PS: I am going to wait few more days for a possible feedback.
    Then I would v4 of this whole patchset with the additional
    changes.

    I hope that the 1st patch from Harry won't need more changes.
    So, I will only fix my part of the patchset. /o\

Best Regards,
Petr

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

[Harry Hsu]

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>
---
This is the selftest I promised in the v2 thread [1].

It applies on top of patch 1/3 of the series [2] and does not touch the
rest of it.  Petr, since you are going to post v4 of the whole patchset
anyway, please feel free to fold this in as the last patch.  Otherwise I
am happy to resend it as a separate follow-up once the series lands --
whichever is less work for you.

Tested on arm64 with CONFIG_LIVEPATCH=y:

  # ./test-alias.sh
  TEST: livepatch of two aliased symbols in one object ... ok
  TEST: aliased symbols in a module coming after the livepatch ... ok

[1] https://lore.kernel.org/all/CAPhsuW70RpkZ1ciioSjt6qkQePWyeic_L+98d0h-Ao3ze-TmkA@mail.gmail.com/
[2] https://lore.kernel.org/all/20260830173343.52759-1-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.43.0