From: Joe Lawrence <[email protected]> klp_reloc_needed() returns true for module exports to support late-module patching. However, clone_reloc_klp() unconditionally rejects symbols without a twin (i.e., new references added by the patch), even when the symbol is a known export from Module.symvers.
Relax the check: allow new references to exported symbols by only erroring on !twin when there is no export. The export metadata from Module.symvers provides sufficient context to emit the klp-relocation without a twin. For a module export that isn't sufficient on its own though, as the resulting klp relocation will only be resolved at patch-enable time if the exporting module is loaded. If the original (unpatched) module already depends on the exporting module, the dependency is safe: the module loader ensures the dependency is satisfied before the patched module can be loaded, so the klp relocation target will exist. However, if the patch introduces a reference to a module that the original doesn't depend on, there is no such guarantee. The exporting module could be absent or could be unloaded at any time, leading to a relocation failure or use-after-free. So also add a build-time check: when a new symbol reference (no twin) targets a module export, verify that the original module already has at least one UNDEF symbol resolving to that same exporting module. If not, error out with a diagnostic message. Signed-off-by: Joe Lawrence <[email protected]> Signed-off-by: Josh Poimboeuf <[email protected]> --- tools/objtool/klp-diff.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c index aebe68a401571..0314426abcd8d 100644 --- a/tools/objtool/klp-diff.c +++ b/tools/objtool/klp-diff.c @@ -1294,6 +1294,28 @@ static int convert_reloc_sym(struct elf *elf, struct reloc *reloc) return convert_reloc_secsym_to_sym(elf, reloc); } +/* + * Check if the original module already has a dependency on dep_mod, i.e. it + * already references at least one export from that module. + */ +static bool has_module_dep(struct elfs *e, const char *dep_mod) +{ + struct symbol *sym; + + for_each_sym(e->orig, sym) { + struct export *exp; + + if (!is_undef_sym(sym) || is_weak_sym(sym)) + continue; + + exp = find_export(sym); + if (exp && !strcmp(exp->mod, dep_mod)) + return true; + } + + return false; +} + /* * Convert a regular relocation to a klp relocation (sort of). */ @@ -1313,8 +1335,17 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc, unsigned long sympos; if (!patched_sym->twin) { - ERROR("unexpected klp reloc for new symbol %s", patched_sym->name); - return -1; + if (!export) { + ERROR("unexpected klp reloc for new symbol %s", patched_sym->name); + return -1; + } + + if (strcmp(export->mod, "vmlinux") && + !has_module_dep(e, export->mod)) { + ERROR("%s: new reference to %s (exported by %s) would create an undeclared module dependency", + patched_sym->name, export->sym, export->mod); + return -1; + } } /* -- 2.54.0
