Re: [PATCH v2 4/7] objtool/klp: Explicitly disallow patching or referencing init code/data

[sashiko-bot@kernel.org]

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 <jpoimboe@kernel.org>

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/cover.1785939903.git.jpoimboe@kernel.org?part=4