linux-kernel - Re: [PATCH] objtool: Don't add empty symbols to the rbtree

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] [day] [month] [year] [list]

Date:   Thu, 7 Jan 2021 10:27:33 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Arnd Bergmann <arnd@...nel.org>
Subject: Re: [PATCH] objtool: Don't add empty symbols to the rbtree

On Wed, Jan 06, 2021 at 09:33:20PM -0600, Josh Poimboeuf wrote:
> Building with the Clang assembler shows the following warning:
> 
>   arch/x86/kernel/ftrace_64.o: warning: objtool: missing symbol for insn at offset 0x16
> 
> The Clang assembler strips section symbols.  That ends up giving
> objtool's find_func_containing() much more test coverage than normal.
> Turns out, find_func_containing() doesn't work so well for overlapping
> symbols:
> 
>      2: 000000000000000e     0 NOTYPE  LOCAL  DEFAULT    2 fgraph_trace
>      3: 000000000000000f     0 NOTYPE  LOCAL  DEFAULT    2 trace
>      4: 0000000000000000   165 FUNC    GLOBAL DEFAULT    2 __fentry__
>      5: 000000000000000e     0 NOTYPE  GLOBAL DEFAULT    2 ftrace_stub
> 
> The zero-length NOTYPE symbols are inside __fentry__(), confusing the
> rbtree search for any __fentry__() offset coming after a NOTYPE.
> 
> Try to avoid this problem by not adding zero-length symbols to the
> rbtree.  They're rare and aren't needed in the rbtree anyway.
> 
> One caveat, this actually might not end up being the right fix.
> Non-empty overlapping symbols, if they exist, could have the same
> problem.  But that would need bigger changes, let's see if we can get
> away with the easy fix for now.

Right, overlapping things needs a different data structure, could be
done though, but like you say, moar work.

> Reported-by: Arnd Bergmann <arnd@...nel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>

> ---
>  tools/objtool/elf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index be89c741ba9a..ccee8fc331f0 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -448,6 +448,13 @@ static int read_symbols(struct elf *elf)
>  		list_add(&sym->list, entry);
>  		elf_hash_add(elf->symbol_hash, &sym->hash, sym->idx);
>  		elf_hash_add(elf->symbol_name_hash, &sym->name_hash, str_hash(sym->name));
> +
> +		/*
> +		 * Don't store empty STT_NOTYPE symbols in the rbtree.  They
> +		 * can exist within a real function, confusing the sorting.
> +		 */
> +		if (!sym->len)
> +			rb_erase(&sym->node, &sym->sec->symbol_tree);
>  	}
>  
>  	if (stats)
> -- 
> 2.29.2
>