From: Song Liu song@kernel.org
[ Upstream commit 139560e8b973402140cafeb68c656c1374bd4c20 ]
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, 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 Acked-by: Josh Poimboeuf jpoimboe@kernel.org [pmladek@suse.com: Fixed some typos.] Reviewed-by: Petr Mladek pmladek@suse.com Tested-by: Petr Mladek pmladek@suse.com Signed-off-by: Petr Mladek pmladek@suse.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
**Subject:** `livepatch: Match old_sympos 0 and 1 in klp_find_func()`
**Key indicators:** - Fixes a bug: livepatch upgrade failures with replace flag - Real-world impact: sysfs duplicate filename error causing patch enablement to fail - Clear problem description with error logs - Tested-by: Petr Mladek - Reviewed-by: Petr Mladek, Josh Poimboeuf
**Root cause:** Different versions of kpatch-build assign `old_sympos` 0 vs 1 for unique functions. During livepatch upgrade with replace, `klp_find_func()` doesn't match them, leading to duplicate sysfs entries.
### 2. CODE CHANGE ANALYSIS
**Files changed:** 1 file (`kernel/livepatch/core.c`) **Lines changed:** 7 lines added, 1 line modified
**Change:** ```c // Before: if ((strcmp(old_func->old_name, func->old_name) == 0) && (old_func->old_sympos == func->old_sympos)) {
// After: if ((strcmp(old_func->old_name, func->old_name) == 0) && ((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))) { ```
**Technical explanation:** - For unique symbols, `old_sympos` 0 and 1 both refer to the first occurrence - `klp_find_object_symbol()` treats `sympos == 0` as "unique symbol" (see lines 170-175) - Sysfs displays `old_sympos == 0` as `1` (line 820: `func->old_sympos ? func->old_sympos : 1`) - The fix makes `klp_find_func()` treat 0 and 1 as equivalent for matching
**Why this fixes the bug:** - During replace upgrade, `klp_add_object_nops()` calls `klp_find_func()` to check if a function already exists - Without the fix, `old_sympos==0` and `old_sympos==1` don't match - This causes duplicate sysfs entries, leading to `-EEXIST` and patch enablement failure
### 3. CLASSIFICATION
**Type:** Bug fix (not a feature)
**Exception categories:** None needed — this is a bug fix
**Security:** No security impact, but prevents a reliability issue
### 4. SCOPE AND RISK ASSESSMENT
**Scope:** - Single function (`klp_find_func()`) - Localized change - No architectural changes
**Risk:** Low - Small, targeted change - Clear logic - No new code paths - Only affects matching logic for unique symbols
**Potential issues:** - None identified - Change is conservative (adds equivalence, doesn't remove checks)
### 5. USER IMPACT
**Severity:** High for affected users - Prevents livepatch upgrades with replace flag - Causes kernel errors and patch enablement failure - Affects users upgrading between different kpatch-build versions
**Affected users:** - Users performing livepatch upgrades with replace - Users mixing kpatch-build versions - Enterprise users relying on livepatch for updates
**Impact assessment:** - Core livepatch functionality (upgrade path) - Real-world scenario (different toolchain versions) - User-visible failure (error messages, failed upgrades)
### 6. STABILITY INDICATORS
**Testing:** - Tested-by: Petr Mladek - Reviewed-by: Petr Mladek, Josh Poimboeuf - Real-world scenario documented
**Code maturity:** - Function exists since atomic replace (Jan 2019) - Present in stable trees (5.1+) - Mature code path
### 7. DEPENDENCY CHECK
**Dependencies:** 1. `klp_find_func()` — introduced with atomic replace (commit e1452b607c48c, Jan 2019) 2. `klp_add_object_nops()` — same commit 3. Replace functionality — present since ~5.1
**Backport compatibility:** - Applies cleanly to any stable tree with atomic replace - No other commits required - Self-contained fix
**Affected stable versions:** - Any stable tree with atomic replace (~5.1+) - All current LTS trees (6.1.y, 6.6.y, etc.)
### 8. STABLE KERNEL RULES COMPLIANCE
**Meets criteria:** 1. Obviously correct: Yes — clear equivalence for unique symbols 2. Fixes real bug: Yes — documented failure case 3. Important issue: Yes — breaks livepatch upgrades 4. Small and contained: Yes — 7 lines, 1 function 5. No new features: Yes — bug fix only 6. Applies cleanly: Yes — no conflicts expected
**No violations:** - No new features - No API changes - No architectural changes - No new dependencies
### 9. RISK VS BENEFIT TRADE-OFF
**Benefit:** - Fixes upgrade failures - Low risk, high value - Addresses real user issue
**Risk:** - Minimal — localized change - Conservative logic - Well-tested
**Conclusion:** Strong benefit, minimal risk
### 10. FINAL ASSESSMENT
This commit should be backported to stable kernel trees.
**Reasons:** 1. Fixes a real bug that breaks livepatch upgrades 2. Small, localized change (7 lines) 3. Low risk, clear logic 4. Well-tested and reviewed 5. No new features or dependencies 6. Applies cleanly to stable trees with atomic replace 7. High user impact for affected users
**Recommendation:** Backport to all stable trees that include atomic replace functionality (approximately 5.1.y and later).
**YES**
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 0e73fac55f8eb..4e7a5cbc40a91 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 consider 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; } }