Re: [PATCH v2] objtool: Fix symbol creation

Josh Poimboeuf
2022-05-19T15:14:27Z

On Thu, May 19, 2022 at 11:00:29AM +0200, Peter Zijlstra wrote:
> Subject: objtool: Fix symbol creation
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue, 17 May 2022 17:42:04 +0200
>
> Nathan reported objtool failing with the following messages:
>
> warning: objtool: no non-local symbols !?
> warning: objtool: gelf_update_symshndx: invalid section index
>
> The problem is due to commit 4abff6d48dbc ("objtool: Fix code relocs
> vs weak symbols") failing to consider the case where an object would
> have no non-local symbols.
>
> The problem that commit tries to address is adding a STB_LOCAL symbol
> to the symbol table in light of the ELF spec's requirement that:
>
> In each symbol table, all symbols with STB_LOCAL binding preced the
> weak and global symbols. As ``Sections'' above describes, a symbol
> table section's sh_info section header member holds the symbol table
> index for the first non-local symbol.
>
> The approach taken is to find this first non-local symbol, move that
> to the end and then re-use the freed spot to insert a new local symbol
> and increment sh_info.
>
> Except it never considered the case of object files without global
> symbols and got a whole bunch of details wrong -- so many in fact that
> it is a wonder it ever worked :/
>
> Specifically:
>
> - It failed to re-hash the symbol on the new index, so a subsequent
> find_symbol_by_index() would not find it at the new location and a
> query for the old location would now return a non-deterministic
> choice between the old and new symbol.
>
> - It failed to appreciate that the GElf wrappers are not a valid disk
> format (it works because GElf is basically Elf64 and we only
> support x86_64 atm.)
>
> - It failed to fully appreciate how horrible the libelf API really is
> and got the gelf_update_symshndx() call pretty much completely
> wrong; with the direct consequence that if inserting a second
> STB_LOCAL symbol would require moving the same STB_GLOBAL symbol
> again it would completely come unstuck.
>
> Write a new elf_update_symbol() function that wraps all the magic
> required to update or create a new symbol at a given index.
>
> Specifically, gelf_update_sym*() require an @ndx argument that is
> relative to the @data argument; this means you have to manually
> iterate the section data descriptor list and update @ndx.
>
> Fixes: 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols")
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

--
Josh