| 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
| ||
|
Message-ID: <20220518074042.GA10117@worktop.programming.kicks-ass.net>
Date: Wed, 18 May 2022 09:40:42 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
llvm@...ts.linux.dev, linux-kernel@...r.kernel.org,
kasan-dev@...glegroups.com
Subject: Re: objtool "no non-local symbols" error with tip of tree LLVM
On Tue, May 17, 2022 at 06:24:29PM -0700, Josh Poimboeuf wrote:
> On Tue, May 17, 2022 at 05:42:04PM +0200, Peter Zijlstra wrote:
> > + for (;;) {
> > + symtab_data = elf_getdata(s, symtab_data);
> > + if (t)
> > + shndx_data = elf_getdata(t, shndx_data);
> >
> > - sym->idx = symtab->sh.sh_size / sizeof(sym->sym);
> > - elf_dirty_reloc_sym(elf, sym);
> > + if (!symtab_data) {
> > + if (!idx) {
> > + void *buf;
>
> I'm confused by whatever this is doing, how is !symtab_data possible,
> i.e. why would symtab not have data?
>
> > 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;
> > + struct symbol *sym, *old;
> > + int size;
> > +
> > + if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
> > + size = sizeof(Elf32_Sym);
> > + else
> > + size = sizeof(Elf64_Sym);
>
> This should probably be called 'entsize' and I think you can just get it
> from symtab->sh.sh_entsize.
>
> > + /*
> > + * Either way, we added a LOCAL symbol.
> > + */
> > + symtab->sh.sh_info += 1;
> > +
> > elf_add_symbol(elf, sym);
>
> Not sure if it matters here, but elf_add_symbol() doesn't set sym->alias
> and sym->pv_target, and both of those are unconditionally initialized in
> read_symbols(). Should elf_add_symbol() be changed to initialize them?
--- 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);
@@ -438,8 +441,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;
@@ -604,7 +605,8 @@ static void elf_dirty_reloc_sym(struct e
/*
* The libelf API is terrible; gelf_update_sym*() takes a data block relative
- * index value. As such, iterate the data blocks and adjust index until it fits.
+ * 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.
@@ -613,14 +615,10 @@ static int elf_update_symbol(struct elf
struct section *symtab_shndx, struct symbol *sym)
{
Elf_Data *symtab_data = NULL, *shndx_data = NULL;
+ Elf64_Xword entsize = symtab->sh.sh_entsize;
Elf32_Word shndx = sym->sec->idx;
Elf_Scn *s, *t = NULL;
- int size, idx = sym->idx;
-
- if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
- size = sizeof(Elf32_Sym);
- else
- size = sizeof(Elf64_Sym);
+ int max_idx, idx = sym->idx;
s = elf_getscn(elf->elf, symtab->idx);
if (!s) {
@@ -637,11 +635,14 @@ static int elf_update_symbol(struct elf
}
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) {
+ /* if @idx == 0, it's the next contiguous entry, create it */
if (!idx) {
void *buf;
@@ -649,53 +650,60 @@ static int elf_update_symbol(struct elf
if (t)
shndx_data = elf_newdata(t);
- buf = calloc(1, size);
+ buf = calloc(1, entsize);
if (!buf) {
WARN("malloc");
return -1;
}
symtab_data->d_buf = buf;
- symtab_data->d_size = size;
+ symtab_data->d_size = entsize;
symtab_data->d_align = 1;
symtab_data->d_type = ELF_T_SYM;
- symtab->sh.sh_size += size;
+ 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 = 4;
+ shndx_data->d_align = sizeof(Elf32_Word);
shndx_data->d_type = ELF_T_WORD;
- symtab_shndx->sh.sh_size += 4;
+ symtab_shndx->sh.sh_size += sizeof(Elf32_Word);
symtab_shndx->changed = true;
}
break;
}
+ /* we don't do holes in symbol tables */
WARN("index out of range");
return -1;
}
+ /* empty blocks should not happen */
if (!symtab_data->d_size) {
WARN("zero size data");
return -1;
}
- if (idx * size < symtab_data->d_size)
+ /* is this the right block? */
+ max_idx = symtab_data->d_size / entsize;
+ if (idx < max_idx)
break;
- idx -= symtab_data->d_size / size;
+ /* 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)
@@ -720,14 +728,8 @@ static struct symbol *
elf_create_section_symbol(struct elf *elf, struct section *sec)
{
struct section *symtab, *symtab_shndx;
- Elf32_Word first_non_local, new;
+ Elf32_Word first_non_local, new_idx;
struct symbol *sym, *old;
- int size;
-
- if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
- size = sizeof(Elf32_Sym);
- else
- size = sizeof(Elf64_Sym);
symtab = find_section_by_name(elf, ".symtab");
if (symtab) {
@@ -752,16 +754,15 @@ elf_create_section_symbol(struct elf *el
// st_value 0
// st_size 0
- new = symtab->sh.sh_size / size;
-
/*
* 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;
+ old->idx = new_idx;
hlist_del(&old->hash);
elf_hash_add(symbol, &old->hash, old->idx);
@@ -773,10 +774,10 @@ elf_create_section_symbol(struct elf *el
return NULL;
}
- new = first_non_local;
+ new_idx = first_non_local;
}
- sym->idx = new;
+ sym->idx = new_idx;
if (elf_update_symbol(elf, symtab, symtab_shndx, sym)) {
WARN("elf_update_symbol");
return NULL;
Powered by blists - more mailing lists