[PATCH AUTOSEL 6.18-5.10] livepatch: Match old_sympos 0 and 1 in klp_find_func()

[Sasha Levin <sashal@kernel.org>]

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;
 		}
 	}
-- 
2.51.0