linux-kernel - Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yc2Tqc69W9ukKDI1@zn.tnic>
Date:   Thu, 30 Dec 2021 12:10:33 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
Cc:     linux-hardening@...r.kernel.org, x86@...nel.org,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Kristen Carlson Accardi <kristen@...ux.intel.com>,
        Kees Cook <keescook@...omium.org>,
        Miklos Szeredi <miklos@...redi.hu>,
        Ard Biesheuvel <ardb@...nel.org>,
        Tony Luck <tony.luck@...el.com>,
        Bruce Schlobohm <bruce.schlobohm@...el.com>,
        Jessica Yu <jeyu@...nel.org>,
        kernel test robot <lkp@...el.com>,
        Miroslav Benes <mbenes@...e.cz>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        Jonathan Corbet <corbet@....net>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>, Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Arnd Bergmann <arnd@...db.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Marios Pomonis <pomonis@...gle.com>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        "H.J. Lu" <hjl.tools@...il.com>, Nicolas Pitre <nico@...xnic.net>,
        linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
        linux-arch@...r.kernel.org, live-patching@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available
 to nuke pos-based search

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.

...

> @@ -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