From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
Signed-off-by: Borislav Petkov <bp@xxxxxxx>
Acked-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Link: https://lkml.kernel.org/r/YoPCTEYjoPqE4ZxB@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
tools/objtool/elf.c | 196 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 128 insertions(+), 68 deletions(-)
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -314,6 +314,8 @@ static void elf_add_symbol(struct elf *e
struct list_head *entry;
struct rb_node *pnode;
+ sym->alias = sym;
+
sym->type = GELF_ST_TYPE(sym->sym.st_info);
sym->bind = GELF_ST_BIND(sym->sym.st_info);
@@ -375,7 +377,6 @@ static int read_symbols(struct elf *elf)
return -1;
}
memset(sym, 0, sizeof(*sym));
- sym->alias = sym;
sym->idx = i;
@@ -539,24 +540,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) {
@@ -564,79 +562,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;
@@ -646,24 +689,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;
