From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3224CCA48C for ; Wed, 8 Jun 2022 00:58:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1441994AbiFHAxj (ORCPT ); Tue, 7 Jun 2022 20:53:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1379944AbiFGWLz (ORCPT ); Tue, 7 Jun 2022 18:11:55 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DB183259F55; Tue, 7 Jun 2022 12:19:08 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id B80ADB823CE; Tue, 7 Jun 2022 19:18:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26F46C34115; Tue, 7 Jun 2022 19:18:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1654629532; bh=Wbu9QY/RGG+sXYd4xeirkbf5jNFHONniUP8DAZ46mGU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fjJTzbaFEVfZp0jAbdcf3cJLgr7rl3aGUBy9OqGlKwlSjC+rqQzAWahvZ4SdOG4Uf cVUfEelnQKF9VCCyruIsQLE3ZOLXKaytUvdWHPP2oWOBHVoUT8CqpnypK2uA6izmT4 OiXkNh6fi/K2DgCsypr0bea7wPb4/HaYHE1oEO60= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Nathan Chancellor , "Peter Zijlstra (Intel)" , Borislav Petkov , Josh Poimboeuf Subject: [PATCH 5.18 718/879] objtool: Fix symbol creation Date: Tue, 7 Jun 2022 19:03:56 +0200 Message-Id: <20220607165023.695240593@linuxfoundation.org> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220607165002.659942637@linuxfoundation.org> References: <20220607165002.659942637@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Peter Zijlstra commit ead165fa1042247b033afad7be4be9b815d04ade upstream. 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 Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Borislav Petkov Acked-by: Josh Poimboeuf Tested-by: Nathan Chancellor Cc: Link: https://lkml.kernel.org/r/YoPCTEYjoPqE4ZxB@hirez.programming.kicks-ass.net Signed-off-by: Greg Kroah-Hartman --- tools/objtool/elf.c | 198 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 129 insertions(+), 69 deletions(-) --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -374,6 +374,9 @@ static void elf_add_symbol(struct elf *e struct list_head *entry; struct rb_node *pnode; + INIT_LIST_HEAD(&sym->pv_target); + sym->alias = sym; + sym->type = GELF_ST_TYPE(sym->sym.st_info); sym->bind = GELF_ST_BIND(sym->sym.st_info); @@ -435,8 +438,6 @@ static int read_symbols(struct elf *elf) return -1; } memset(sym, 0, sizeof(*sym)); - INIT_LIST_HEAD(&sym->pv_target); - sym->alias = sym; sym->idx = i; @@ -600,24 +601,21 @@ static void elf_dirty_reloc_sym(struct e } /* - * Move the first global symbol, as per sh_info, into a new, higher symbol - * index. This fees up the shndx for a new local symbol. + * The libelf API is terrible; gelf_update_sym*() takes a data block relative + * index value, *NOT* the symbol index. As such, iterate the data blocks and + * adjust index until it fits. + * + * If no data block is found, allow adding a new data block provided the index + * is only one past the end. */ -static int elf_move_global_symbol(struct elf *elf, struct section *symtab, - struct section *symtab_shndx) +static int elf_update_symbol(struct elf *elf, struct section *symtab, + struct section *symtab_shndx, struct symbol *sym) { - Elf_Data *data, *shndx_data = NULL; - Elf32_Word first_non_local; - struct symbol *sym; - Elf_Scn *s; - - first_non_local = symtab->sh.sh_info; - - sym = find_symbol_by_index(elf, first_non_local); - if (!sym) { - WARN("no non-local symbols !?"); - return first_non_local; - } + Elf32_Word shndx = sym->sec ? sym->sec->idx : SHN_UNDEF; + Elf_Data *symtab_data = NULL, *shndx_data = NULL; + Elf64_Xword entsize = symtab->sh.sh_entsize; + int max_idx, idx = sym->idx; + Elf_Scn *s, *t = NULL; s = elf_getscn(elf->elf, symtab->idx); if (!s) { @@ -625,79 +623,124 @@ static int elf_move_global_symbol(struct return -1; } - data = elf_newdata(s); - if (!data) { - WARN_ELF("elf_newdata"); - return -1; + if (symtab_shndx) { + t = elf_getscn(elf->elf, symtab_shndx->idx); + if (!t) { + WARN_ELF("elf_getscn"); + return -1; + } } - data->d_buf = &sym->sym; - data->d_size = sizeof(sym->sym); - data->d_align = 1; - data->d_type = ELF_T_SYM; + for (;;) { + /* get next data descriptor for the relevant sections */ + symtab_data = elf_getdata(s, symtab_data); + if (t) + shndx_data = elf_getdata(t, shndx_data); + + /* end-of-list */ + if (!symtab_data) { + void *buf; + + if (idx) { + /* we don't do holes in symbol tables */ + WARN("index out of range"); + return -1; + } - sym->idx = symtab->sh.sh_size / sizeof(sym->sym); - elf_dirty_reloc_sym(elf, sym); + /* if @idx == 0, it's the next contiguous entry, create it */ + symtab_data = elf_newdata(s); + if (t) + shndx_data = elf_newdata(t); + + buf = calloc(1, entsize); + if (!buf) { + WARN("malloc"); + return -1; + } - symtab->sh.sh_info += 1; - symtab->sh.sh_size += data->d_size; - symtab->changed = true; + symtab_data->d_buf = buf; + symtab_data->d_size = entsize; + symtab_data->d_align = 1; + symtab_data->d_type = ELF_T_SYM; + + symtab->sh.sh_size += entsize; + symtab->changed = true; + + if (t) { + shndx_data->d_buf = &sym->sec->idx; + shndx_data->d_size = sizeof(Elf32_Word); + shndx_data->d_align = sizeof(Elf32_Word); + shndx_data->d_type = ELF_T_WORD; - if (symtab_shndx) { - s = elf_getscn(elf->elf, symtab_shndx->idx); - if (!s) { - WARN_ELF("elf_getscn"); + symtab_shndx->sh.sh_size += sizeof(Elf32_Word); + symtab_shndx->changed = true; + } + + break; + } + + /* empty blocks should not happen */ + if (!symtab_data->d_size) { + WARN("zero size data"); return -1; } - shndx_data = elf_newdata(s); + /* is this the right block? */ + max_idx = symtab_data->d_size / entsize; + if (idx < max_idx) + break; + + /* adjust index and try again */ + idx -= max_idx; + } + + /* something went side-ways */ + if (idx < 0) { + WARN("negative index"); + return -1; + } + + /* setup extended section index magic and write the symbol */ + if (shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) { + sym->sym.st_shndx = shndx; + if (!shndx_data) + shndx = 0; + } else { + sym->sym.st_shndx = SHN_XINDEX; if (!shndx_data) { - WARN_ELF("elf_newshndx_data"); + WARN("no .symtab_shndx"); return -1; } + } - shndx_data->d_buf = &sym->sec->idx; - shndx_data->d_size = sizeof(Elf32_Word); - shndx_data->d_align = 4; - shndx_data->d_type = ELF_T_WORD; - - symtab_shndx->sh.sh_size += 4; - symtab_shndx->changed = true; + if (!gelf_update_symshndx(symtab_data, shndx_data, idx, &sym->sym, shndx)) { + WARN_ELF("gelf_update_symshndx"); + return -1; } - return first_non_local; + return 0; } static struct symbol * elf_create_section_symbol(struct elf *elf, struct section *sec) { struct section *symtab, *symtab_shndx; - Elf_Data *shndx_data = NULL; - struct symbol *sym; - Elf32_Word shndx; + Elf32_Word first_non_local, new_idx; + struct symbol *sym, *old; symtab = find_section_by_name(elf, ".symtab"); if (symtab) { symtab_shndx = find_section_by_name(elf, ".symtab_shndx"); - if (symtab_shndx) - shndx_data = symtab_shndx->data; } else { WARN("no .symtab"); return NULL; } - sym = malloc(sizeof(*sym)); + sym = calloc(1, sizeof(*sym)); if (!sym) { perror("malloc"); return NULL; } - memset(sym, 0, sizeof(*sym)); - - sym->idx = elf_move_global_symbol(elf, symtab, symtab_shndx); - if (sym->idx < 0) { - WARN("elf_move_global_symbol"); - return NULL; - } sym->name = sec->name; sym->sec = sec; @@ -707,24 +750,41 @@ elf_create_section_symbol(struct elf *el // st_other 0 // st_value 0 // st_size 0 - shndx = sec->idx; - if (shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) { - sym->sym.st_shndx = shndx; - if (!shndx_data) - shndx = 0; - } else { - sym->sym.st_shndx = SHN_XINDEX; - if (!shndx_data) { - WARN("no .symtab_shndx"); + + /* + * Move the first global symbol, as per sh_info, into a new, higher + * symbol index. This fees up a spot for a new local symbol. + */ + first_non_local = symtab->sh.sh_info; + new_idx = symtab->sh.sh_size / symtab->sh.sh_entsize; + old = find_symbol_by_index(elf, first_non_local); + if (old) { + old->idx = new_idx; + + hlist_del(&old->hash); + elf_hash_add(symbol, &old->hash, old->idx); + + elf_dirty_reloc_sym(elf, old); + + if (elf_update_symbol(elf, symtab, symtab_shndx, old)) { + WARN("elf_update_symbol move"); return NULL; } + + new_idx = first_non_local; } - if (!gelf_update_symshndx(symtab->data, shndx_data, sym->idx, &sym->sym, shndx)) { - WARN_ELF("gelf_update_symshndx"); + sym->idx = new_idx; + if (elf_update_symbol(elf, symtab, symtab_shndx, sym)) { + WARN("elf_update_symbol"); return NULL; } + /* + * Either way, we added a LOCAL symbol. + */ + symtab->sh.sh_info += 1; + elf_add_symbol(elf, sym); return sym;