lkml.org 
[lkml]   [2022]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.17 619/772] objtool: Fix symbol creation
    Date
    From: Peter Zijlstra <peterz@infradead.org>

    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 <nathan@kernel.org>
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Signed-off-by: Borislav Petkov <bp@suse.de>
    Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
    Tested-by: Nathan Chancellor <nathan@kernel.org>
    Cc: <stable@vger.kernel.org>
    Link: https://lkml.kernel.org/r/YoPCTEYjoPqE4ZxB@hirez.programming.kicks-ass.net
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    tools/objtool/elf.c | 198 +++++++++++++++++++++++++++++++++-------------------
    1 file changed, 129 insertions(+), 69 deletions(-)

    --- a/tools/objtool/elf.c
    +++ b/tools/objtool/elf.c
    @@ -314,6 +314,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);

    @@ -375,8 +378,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;

    @@ -540,24 +541,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) {
    @@ -565,79 +563,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;
    @@ -647,24 +690,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;

    \
     
     \ /
      Last update: 2022-06-07 23:37    [W:2.545 / U:1.632 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site