Re: [PATCH] objtool: Fix symbol creation

Sami Tolvanen
2022-09-07T00:48:05Z

On Wed, May 18, 2022 at 3:10 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 18, 2022 at 10:36:04AM -0700, Josh Poimboeuf wrote:
> > On Wed, May 18, 2022 at 09:41:52AM +0200, Peter Zijlstra wrote:
> > > +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;
> > > - }
> > > + Elf_Data *symtab_data = NULL, *shndx_data = NULL;
> > > + Elf64_Xword entsize = symtab->sh.sh_entsize;
> > > + Elf32_Word shndx = sym->sec->idx;
> >
> > So if it's a global UNDEF symbol then I think 'sym->sec' can be NULL and
> > this blows up?
>
> Oh indeed, sym->sec ? sym->sec->idx : SHN_UNDEF it is.

elf_update_symbol seems to be a bit broken even after this. I noticed
it converts SHN_ABS symbols into SHN_UNDEF, which breaks some KCFI
builds. In fact, the function drops all the special st_shndx values
except SHN_XINDEX.

Specifically, read_symbols sets sym->sec to find_section_by_index(elf,
0) for all SHN_UNDEF and special st_shndx symbols, which means
sym->sec is non-NULL and sym->sec->idx is always 0 (= SHN_UNDEF) for
these symbols. As elf_update_symbol doesn't look at the actual
st_shndx value, it ends up marking the symbols undefined.

This quick hack fixes the issue for me, but I'm not sure if it's the
cleanest solution. Any thoughts?

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index c25e957c1e52..7e24b09b1163 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -619,6 +619,11 @@ static int elf_update_symbol(struct elf *elf,
struct section *symtab,
Elf64_Xword entsize = symtab->sh.sh_entsize;
int max_idx, idx = sym->idx;
Elf_Scn *s, *t = NULL;
+ bool is_special_shndx = sym->sym.st_shndx >= SHN_LORESERVE &&
+ sym->sym.st_shndx != SHN_XINDEX;
+
+ if (is_special_shndx)
+ shndx = sym->sym.st_shndx;

s = elf_getscn(elf->elf, symtab->idx);
if (!s) {
@@ -704,7 +709,7 @@ static int elf_update_symbol(struct elf *elf,
struct section *symtab,
}

/* setup extended section index magic and write the symbol */
- if (shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) {
+ if ((shndx >= SHN_UNDEF && shndx < SHN_LORESERVE) || is_special_shndx) {
sym->sym.st_shndx = shndx;
if (!shndx_data)
shndx = 0;

Sami