linux-kernel - Re: [PATCH] objtool: Fix symbol creation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABCJKueB-tZmxESGP_W9JUghu-6y1Dj1DeahRsGb3bOUttctMA@mail.gmail.com>
Date:   Tue, 6 Sep 2022 17:47:23 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        llvm@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>,
        kasan-dev@...glegroups.com
Subject: Re: [PATCH] objtool: Fix symbol creation

On Wed, May 18, 2022 at 3:10 PM Peter Zijlstra <peterz@...radead.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