[PATCH v2] livepatch: Match old_sympos 0 and 1 in klp_find_func()

[PATCH v2] livepatch: Match old_sympos 0 and 1 in klp_find_func()

[Song Liu]

When there is only one function of the same name, old_sympos of 0 and 1
are logically identical. Match them in klp_find_func().

This is to avoid a corner case with different toolchain behavior.

In this specific issue, issue two versions of kpatch-build were used to
build livepatch for the same kernel. One assigns old_sympos == 0 for
unique local functions, the other assigns old_sympos == 1 for unique
local functions. Both versions work fine by themselves. (PS: This
behavior change was introduced in a downstream version of kpatch-build.
This change does not exist in upstream kpatch-build.)

However, during livepatch upgrade (with the replace flag set) from a
patch built with one version of kpatch-build to the same fix built with
the other version of kpatch-build, livepatching fails with errors like:

[   14.218706] sysfs: cannot create duplicate filename 'xxx/somefunc,1'
...
[   14.219466] Call Trace:
[   14.219468]  <TASK>
[   14.219469]  dump_stack_lvl+0x47/0x60
[   14.219474]  sysfs_warn_dup.cold+0x17/0x27
[   14.219476]  sysfs_create_dir_ns+0x95/0xb0
[   14.219479]  kobject_add_internal+0x9e/0x260
[   14.219483]  kobject_add+0x68/0x80
[   14.219485]  ? kstrdup+0x3c/0xa0
[   14.219486]  klp_enable_patch+0x320/0x830
[   14.219488]  patch_init+0x443/0x1000 [ccc_0_6]
[   14.219491]  ? 0xffffffffa05eb000
[   14.219492]  do_one_initcall+0x2e/0x190
[   14.219494]  do_init_module+0x67/0x270
[   14.219496]  init_module_from_file+0x75/0xa0
[   14.219499]  idempotent_init_module+0x15a/0x240
[   14.219501]  __x64_sys_finit_module+0x61/0xc0
[   14.219503]  do_syscall_64+0x5b/0x160
[   14.219505]  entry_SYSCALL_64_after_hwframe+0x4b/0x53
[   14.219507] RIP: 0033:0x7f545a4bd96d
...
[   14.219516] kobject: kobject_add_internal failed for somefunc,1 with
    -EEXIST, don't try to register things with the same name ...

This happens because klp_find_func() thinks somefunc with old_sympos==0
is not the same as somefunc with old_sympos==1, and klp_add_object_nops
adds another xxx/func,1 to the list of functions to patch.

Signed-off-by: Song Liu <song@kernel.org>
---
 kernel/livepatch/core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 0e73fac55f8e..53c3b9b40c8b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -88,8 +88,14 @@ static struct klp_func *klp_find_func(struct klp_object *obj,
 	struct klp_func *func;
 
 	klp_for_each_func(obj, func) {
+		/*
+		 * Besides identical old_sympos, also condiser old_sympos
+		 * of 0 and 1 are identical.
+		 */
 		if ((strcmp(old_func->old_name, func->old_name) == 0) &&
-		    (old_func->old_sympos == func->old_sympos)) {
+		    ((old_func->old_sympos == func->old_sympos) ||
+		     (old_func->old_sympos == 0 && func->old_sympos == 1) ||
+		     (old_func->old_sympos == 1 && func->old_sympos == 0))) {
 			return func;
 		}
 	}
-- 
2.47.3

Re: [PATCH v2] livepatch: Match old_sympos 0 and 1 in klp_find_func()

[Josh Poimboeuf]

On Mon, Oct 13, 2025 at 10:30:19AM -0700, Song Liu wrote:
> +++ b/kernel/livepatch/core.c
> @@ -88,8 +88,14 @@ static struct klp_func *klp_find_func(struct klp_object *obj,
>  	struct klp_func *func;
>  
>  	klp_for_each_func(obj, func) {
> +		/*
> +		 * Besides identical old_sympos, also condiser old_sympos
> +		 * of 0 and 1 are identical.

"consider"

Otherwise, seems fine...

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh

Re: [PATCH v2] livepatch: Match old_sympos 0 and 1 in klp_find_func()

[Petr Mladek]

On Mon 2025-10-13 10:30:19, Song Liu wrote:
> When there is only one function of the same name, old_sympos of 0 and 1
> are logically identical. Match them in klp_find_func().
> 
> This is to avoid a corner case with different toolchain behavior.
> 
> In this specific issue, issue two versions of kpatch-build were used to

  s/issue, issue/issue/

> build livepatch for the same kernel. One assigns old_sympos == 0 for
> unique local functions, the other assigns old_sympos == 1 for unique
> local functions. Both versions work fine by themselves. (PS: This
> behavior change was introduced in a downstream version of kpatch-build.
> This change does not exist in upstream kpatch-build.)
> 
> However, during livepatch upgrade (with the replace flag set) from a
> patch built with one version of kpatch-build to the same fix built with
> the other version of kpatch-build, livepatching fails with errors like:
> 
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -88,8 +88,14 @@ static struct klp_func *klp_find_func(struct klp_object *obj,
>  	struct klp_func *func;
>  
>  	klp_for_each_func(obj, func) {
> +		/*
> +		 * Besides identical old_sympos, also condiser old_sympos

As Josh already pointed out:

   s/condiser/consider/

> +		 * of 0 and 1 are identical.
> +		 */
>  		if ((strcmp(old_func->old_name, func->old_name) == 0) &&
> -		    (old_func->old_sympos == func->old_sympos)) {
> +		    ((old_func->old_sympos == func->old_sympos) ||
> +		     (old_func->old_sympos == 0 && func->old_sympos == 1) ||
> +		     (old_func->old_sympos == 1 && func->old_sympos == 0))) {
>  			return func;
>  		}
>  	}

With the two above fixes:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

I have fixed both problems when committing.

And the patch has been committed into livepatching.git,
branch for-6.19/trivial.

Best Regards,
Petr