Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
From: Borislav Petkov <bp@alien8.de>
Date: 2021-12-30 11:10:37
Also in:
linux-arch, linux-hardening, linux-kbuild, lkml, llvm
On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
[PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
nuke? I think you wanna say something about avoiding position-based search if toolchain supports -z ...
Position-based search, which means that if we have several symbols with the same name, we additionally need to provide an "index" of the desired symbol, is fragile. Par exemple, it breaks when two
^^^^^^^^^^^^ We already have hard time with the English in commit messages, let's avoid the French pls.
symbols with the same name are located in different sections. Since a while, LD has a flag `-z unique-symbol` which appends numeric suffixes to the functions with the same name (in symtab and strtab). Check for its availability and always prefer when the livepatching is on.
Why only then? It looks to me like we want this unconditionally, no?
This needs a little adjustment to the modpost to make it strip suffixes before adding exports. depmod needs some treatment as well, tho its false-positibe warnings
Unknown word [false-positibe] in commit message, suggestions:
['false-positive', 'false-positioned', 'prepositional']
Please introduce a spellchecker into your patch creation workflow.
about unknown symbols are harmless and don't alter the return code. And there is a bunch more livepatch code to optimize-out after introducing this, but let's leave it for later.
...
quoted hunk ↗ jump to hunk
@@ -171,17 +173,21 @@ static int klp_find_object_symbol(const char *objname, const char *name, /* * Ensure an address was found. If sympos is 0, ensure symbol is unique; - * otherwise ensure the symbol position count matches sympos. + * otherwise ensure the symbol position count matches sympos. If the LD + * `-z unique` flag is enabled, sympos checks are not relevant.
^^^^^^^^^^^ -z unique-symbol
*/
- if (args.addr == 0)
+ if (args.addr == 0) {
pr_err("symbol '%s' not found in symbol table\n", name);
- else if (args.count > 1 && sympos == 0) {
+ } else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
+ goto out_ok;This is silly - just do it all here.
+ } else if (args.count > 1 && sympos == 0) {
pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
name, objname);
} else if (sympos != args.count && sympos > 0) {
pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
sympos, name, objname ? objname : "vmlinux");
} else {
+out_ok:
*addr = args.addr;
return 0;
}
Looks straight-forward otherwise but I'm no livepatcher so I'd prefer if
they have a look too.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette