livepatch: Match old_sympos 0 and 1 in klp_find_func()
Song Liu
maintainerPATCH v2 1/1 livepatch: Match old_sympos 0 and 1 in klp_find_func()
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.
--- kernel/livepatch/core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
kernel/livepatch/core.c +7 -1
@@ -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; } }
Signature · 1 line
2.47.3
kernel/livepatch/core.c static struct klp_func *klp_find_func(struct klp_object *obj, · line 93
Quoted 8 lines · Song Liu
+++
b/kernel/livepatch/core.ckernel/livepatch/core.c +3
-0@@ -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...
Signature · 1 line
Josh
Quoted 6 lines · Song Liu
When there is only one function of the same name,
old_symposof 0 and 1 are logically identical. Match them inklp_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/
Quoted 13 lines
build livepatch for the same kernel. One assigns
old_sympos== 0 for unique local functions, the other assignsold_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:
kernel/livepatch/core.c static struct klp_func *klp_find_func(struct klp_object *obj, · line 92
Quoted 5 lines
kernel/livepatch/core.c +2
-0@@ -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/
kernel/livepatch/core.c static struct klp_func *klp_find_func(struct klp_object *obj, · line 101
Quoted 10 lines
kernel/livepatch/core.c +5
-1+* 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:
I have fixed both problems when committing.
And the patch has been committed into livepatching.git, branch for-6.19/trivial.
Best Regards,
Petr