Re: [PATCH 1/1] objtool/klp: fix symbol ordering for data symbols
From: sashiko-bot@kernel.org
Date: 2026-07-10 15:40:38
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] The new multi-pass sympos calculation for data symbols hardcodes an incorrect section ordering that prioritizes `.data..ro_after_init` over `.rodata`, and lumps other distinct data sections together, failing to match kallsyms address ordering. Pre-existing issues: - [High] Calculating `sympos` for cross-type duplicates (e.g., a function and a variable sharing the same name) yields contradictory results because the iteration passes diverge based on the queried symbol's type without filtering the counted symbols. --
--- Patch [1]: [PATCH 1/1] objtool/klp: fix symbol ordering for data symbols ---commit c98091d5ce8ba2d91968d3bbc6e227ed2bf27934 Author: Joe Lawrence [off-list ref] objtool/klp: fix symbol ordering for data symbols The find_sympos() function calculates a sympos (symbol position) used by livepatch to disambiguate duplicate symbol names. This commit extends the existing section ordering workaround to handle data symbols, counting .data..ro_after_init first, then other .data, then .bss.
quoted hunk ↗ jump to hunk
diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c index f8787d7d14547..034778885e922 100644 --- a/tools/objtool/klp-diff.c +++ b/tools/objtool/klp-diff.c@@ -873,6 +873,8 @@ static unsigned long find_sympos(struct elf *elf, struct symbol *sym) if (sym->bind != STB_LOCAL) return 0; if (vmlinux && is_func_sym(sym)) {
[Severity: High] This is a pre-existing issue, but does this logic cause collisions when a function and a variable share the same name? The branches here decide which sections are prioritized during counting based on the queried symbol's type (is_func_sym vs is_object_sym). However, the inner loops count all duplicates blindly by name without filtering the counted symbols by type. If there is a cross-type duplicate, the function and the variable could each be found first during their respective queries because the passes iterate sections in different orders. Would this result in both getting the exact same sympos? Since klp_find_object_symbol doesn't filter by type in the kernel, could a request to patch the variable erroneously resolve to the function? [ ... ]
quoted hunk ↗ jump to hunk
@@ -905,6 +907,46 @@ static unsigned long find_sympos(struct elf *elf, struct symbol *sym) has_dup = true; } } + } else if (vmlinux && is_object_sym(sym)) { + /* + * HACK: Similarly, data symbol ordering can differ between + * vmlinux.o and vmlinux. The linker script places sections: + * .data..ro_after_init (lowest address) + * .data* sections + * .bss* sections (highest address) + * Count in that order. + */
[Severity: High] Does this hardcoded section ordering match the actual kallsyms address ordering? In the kernel linker script, .rodata is placed before RO_AFTER_INIT_DATA, meaning .rodata symbols will appear before .data..ro_after_init symbols since kallsyms sorts strictly by address. If a livepatch contains duplicate static data symbols where one is in .rodata and another is in .data..ro_after_init, this patch will count .data..ro_after_init first. Could this assign sympos=1 to the .data..ro_after_init symbol and sympos=2 to the .rodata symbol, causing livepatch to patch the wrong variable when it asks for sympos=2?
+ for_each_sym(elf, s) {
+ if (strstarts(s->sec->name, ".data..ro_after_init") &&
+ !strcmp(s->name, sym->name)) {
+ nr_matches++;-- Sashiko AI review · https://sashiko.dev/#/patchset/20260710153042.3156788-1-joe.lawrence@redhat.com?part=1