On Thu, Jan 14, 2021 at 12:49:28PM -0800, Sami Tolvanen wrote:
> Hi Josh,
>
> On Thu, Jan 14, 2021 at 11:41 AM Josh Poimboeuf <jpoim...@redhat.com> wrote:
> >
> > The upcoming CONFIG_CFI_CLANG support uses -fsanitize=cfi, the
> > non-canonical version of which hijacks function entry by changing
> > function relocation references to point to an intermediary jump table.
> >
> > For example:
> >
> > Relocation section '.rela.discard.func_stack_frame_non_standard' at
> > offset 0x37e018 contains 6 entries:
> > Offset Info Type Symbol's Value
> > Symbol's Name + Addend
> > 0000000000000000 0002944700000002 R_X86_64_PC32
> > 00000000000023f0 do_suspend_lowlevel + 0
> > 0000000000000008 0003c11900000001 R_X86_64_64
> > 0000000000000008 xen_cpuid$e69bc59f4fade3b6f2b579b3934137df.cfi_jt + 0
> > 0000000000000010 0003980900000001 R_X86_64_64
> > 0000000000000060 machine_real_restart.cfi_jt + 0
> > 0000000000000018 0003962b00000001 R_X86_64_64
> > 0000000000000e18 kretprobe_trampoline.cfi_jt + 0
> > 0000000000000020 000028f300000001 R_X86_64_64
> > 0000000000000000 .rodata + 12
> > 0000000000000028 000349f400000001 R_X86_64_64
> > 0000000000000018 __crash_kexec.cfi_jt + 0
> >
> > 0000000000000060 <machine_real_restart.cfi_jt>:
> > 60: e9 00 00 00 00 jmpq 65 <machine_real_restart.cfi_jt+0x5>
> > 61: R_X86_64_PLT32 machine_real_restart-0x4
> > 65: cc int3
> > 66: cc int3
> > 67: cc int3
> >
> > This breaks objtool vmlinux validation in many ways, including static
> > call site detection and the STACK_FRAME_NON_STANDARD() macro.
> >
> > Fix it by converting those relocations' symbol references back to their
> > original non-jump-table versions. Note this doesn't change the actual
> > relocations in the object itself, it just changes objtool's view of
> > them.
> >
> > Reported-by: Sedat Dilek <sedat.di...@gmail.com>
> > Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com>
> > ---
> > tools/objtool/elf.c | 28 ++++++++++++++++++++++++++++
> > tools/objtool/elf.h | 2 +-
> > 2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > index 292f015f7ec6..e357dc34cd7a 100644
> > --- a/tools/objtool/elf.c
> > +++ b/tools/objtool/elf.c
> > @@ -382,6 +382,11 @@ static int read_sections(struct elf *elf)
> > }
> > sec->len = sec->sh.sh_size;
> >
> > + /* Detect -fsanitize=cfi related sections */
> > + if (!strcmp(sec->name, ".text.__cfi_check") ||
> > + !strncmp(sec->name, ".text..L.cfi.jumptable", 22))
> > + sec->cfi_jt = true;
> > +
> > list_add_tail(&sec->list, &elf->sections);
> > elf_hash_add(elf->section_hash, &sec->hash, sec->idx);
> > elf_hash_add(elf->section_name_hash, &sec->name_hash,
> > str_hash(sec->name));
> > @@ -614,6 +619,29 @@ static int read_relocs(struct elf *elf)
> > return -1;
> > }
> >
> > + /*
> > + * Deal with -fsanitize=cfi (CONFIG_CFI_CLANG),
> > which
> > + * hijacks function entry by arbitrarily changing a
> > lot
> > + * of relocation symbol references to refer to an
> > + * intermediate jump table. Undo that conversion so
> > + * objtool can make sense of things.
> > + */
> > + if (reloc->sym->sec->cfi_jt) {
> > + struct symbol *func, *sym;
> > +
> > + if (sym->type == STT_SECTION)
> > + sym = find_func_by_offset(sym->sec,
> > +
> > reloc->addend);
>
> Clang points out that sym is uninitialized here. Should these be
> reloc->sym instead?
Yup, I somehow managed to bork this patch (and the one before it) right
before I sent it. Will send updated versions of these two.
--
Josh
