Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The `.init` safety check is bypassed for exported module symbols, allowing livepatches to silently reference freed init memory. --
--- Patch [4]: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data --- commit fe2fe6cc51b729f1eb6addc3f9e161e818ce482f Author: Josh Poimboeuf <[email protected]> objtool/klp: Explicitly disallow patching or referencing init code/data This commit explicitly prevents the patching and referencing of init code and data to avoid edge cases. It introduces an init check in the sympos resolution to handle targets that might have their init section freed. > diff --git a/tools/objtool/klp-sympos.c b/tools/objtool/klp-sympos.c > index bbfae516d3395..dfca9dd746812 100644 > --- a/tools/objtool/klp-sympos.c > +++ b/tools/objtool/klp-sympos.c [ ... ] > @@ -376,6 +381,11 @@ unsigned long klp_find_sympos(struct elf *elf, struct > symbol *sym) > bool has_dup = false; > struct symbol *s; > > + if (is_init_sym(sym)) { > + ERROR("%s: can't patch or reference init code/data", sym->name); > + return ULONG_MAX; > + } > + [Severity: High] Does this check adequately protect against exported module symbols? Looking at clone_reloc_klp() in tools/objtool/klp-diff.c, if the symbol is exported, the code skips calling klp_find_sympos() entirely: tools/objtool/klp-diff.c:clone_reloc_klp() { if (export) { sym_modname = export->mod; sym_orig_name = export->sym; sympos = 0; } else { sym_modname = find_modname(e); if (!sym_modname) return -1; sym_orig_name = patched_sym->twin->name; sympos = klp_find_sympos(e->orig, patched_sym->twin); ... } If a livepatch object references an exported init symbol from a module, would it bypass this new safety check and potentially result in a reference to freed init memory once the target module finishes initialization? > if (sym->bind != STB_LOCAL) > return 0; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
