| Message ID | 20210914191045.2234020-2-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: Add support for Clang CFI | expand |
On Tue, Sep 14, 2021 at 12:10 PM Sami Tolvanen <samitolvanen@google.com> wrote: > > With CONFIG_CFI_CLANG, the compiler replaces function references with > references to the CFI jump table, which confuses objtool. This change, > based on Josh's initial patch [1], goes through the list of relocations > and replaces jump table symbols with the actual function symbols. > > [1] https://lore.kernel.org/r/d743f4b36e120c06506567a9f87a062ae03da47f.1611263462.git.jpoimboe@redhat.com/ > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > tools/objtool/arch/x86/decode.c | 16 +++++++++ > tools/objtool/elf.c | 51 ++++++++++++++++++++++++++++ > tools/objtool/include/objtool/arch.h | 3 ++ > tools/objtool/include/objtool/elf.h | 2 +- > 4 files changed, 71 insertions(+), 1 deletion(-) > > diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c > index bc821056aba9..318189c8065e 100644 > --- a/tools/objtool/arch/x86/decode.c > +++ b/tools/objtool/arch/x86/decode.c > @@ -62,6 +62,22 @@ bool arch_callee_saved_reg(unsigned char reg) > } > } > > +unsigned long arch_cfi_section_reloc_offset(struct reloc *reloc) > +{ > + if (!reloc->addend) > + return 0; > + > + if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32) > + return reloc->addend + 4; > + > + return reloc->addend; > +} > + > +unsigned long arch_cfi_jump_reloc_offset(unsigned long offset) > +{ > + return offset + 1; > +} > + > unsigned long arch_dest_reloc_offset(int addend) > { > return addend + 4; > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c > index 8676c7598728..05a5f51aad2c 100644 > --- a/tools/objtool/elf.c > +++ b/tools/objtool/elf.c > @@ -18,6 +18,7 @@ > #include <errno.h> > #include <objtool/builtin.h> > > +#include <objtool/arch.h> > #include <objtool/elf.h> > #include <objtool/warn.h> > > @@ -291,6 +292,10 @@ static int read_sections(struct elf *elf) > if (sec->sh.sh_flags & SHF_EXECINSTR) > elf->text_size += sec->len; > > + /* Detect -fsanitize=cfi jump table sections */ > + if (!strncmp(sec->name, ".text..L.cfi.jumptable", 22)) > + sec->cfi_jt = true; > + > list_add_tail(&sec->list, &elf->sections); > elf_hash_add(section, &sec->hash, sec->idx); > elf_hash_add(section_name, &sec->name_hash, str_hash(sec->name)); > @@ -576,6 +581,49 @@ static int read_rela_reloc(struct section *sec, int i, struct reloc *reloc, unsi > return 0; > } > > +/* > + * CONFIG_CFI_CLANG replaces function relocations to refer to an intermediate > + * jump table. Undo the conversion so objtool can make sense of things. > + */ > +static int fix_cfi_relocs(const struct elf *elf) > +{ > + struct section *sec; > + struct reloc *reloc; > + > + list_for_each_entry(sec, &elf->sections, list) { > + list_for_each_entry(reloc, &sec->reloc_list, list) { > + struct reloc *cfi_reloc; > + unsigned long offset; > + > + if (!reloc->sym->sec->cfi_jt) > + continue; > + > + if (reloc->sym->type == STT_SECTION) > + offset = arch_cfi_section_reloc_offset(reloc); > + else > + offset = reloc->sym->offset; > + > + /* > + * The jump table immediately jumps to the actual function, > + * so look up the relocation there. > + */ > + offset = arch_cfi_jump_reloc_offset(offset); Sorry, this comment is curious to me, it looks like we jump to the offset+1, not directly to the actual function? Perhaps a comment above arch_cfi_jump_reloc_offset() and/or amending this comment might make it clearer? Sorry if this is obvious to others? Perhaps comments can be cleaned up in a follow up, if this is not a bug? > + cfi_reloc = find_reloc_by_dest(elf, reloc->sym->sec, offset); > + > + if (!cfi_reloc || !cfi_reloc->sym) { > + WARN("can't find a CFI jump table relocation at %s+0x%lx", > + reloc->sym->sec->name, offset); > + return -1; > + } > + > + reloc->sym = cfi_reloc->sym; > + reloc->addend = 0; > + } > + } > + > + return 0; > +} > + > static int read_relocs(struct elf *elf) > { > struct section *sec; > @@ -639,6 +687,9 @@ static int read_relocs(struct elf *elf) > tot_reloc += nr_reloc; > } > > + if (fix_cfi_relocs(elf)) > + return -1; > + > if (stats) { > printf("max_reloc: %lu\n", max_reloc); > printf("tot_reloc: %lu\n", tot_reloc); > diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h > index 062bb6e9b865..2205b2b08268 100644 > --- a/tools/objtool/include/objtool/arch.h > +++ b/tools/objtool/include/objtool/arch.h > @@ -81,6 +81,9 @@ unsigned long arch_jump_destination(struct instruction *insn); > > unsigned long arch_dest_reloc_offset(int addend); > > +unsigned long arch_cfi_section_reloc_offset(struct reloc *reloc); > +unsigned long arch_cfi_jump_reloc_offset(unsigned long offset); > + > const char *arch_nop_insn(int len); > > int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg); > diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h > index e34395047530..d9c1dacc6572 100644 > --- a/tools/objtool/include/objtool/elf.h > +++ b/tools/objtool/include/objtool/elf.h > @@ -39,7 +39,7 @@ struct section { > char *name; > int idx; > unsigned int len; > - bool changed, text, rodata, noinstr; > + bool changed, text, rodata, noinstr, cfi_jt; > }; > > struct symbol { > -- > 2.33.0.309.g3052b89438-goog >
On Tue, Sep 14, 2021 at 12:29 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Tue, Sep 14, 2021 at 12:10 PM Sami Tolvanen <samitolvanen@google.com> wrote: > > > > With CONFIG_CFI_CLANG, the compiler replaces function references with > > references to the CFI jump table, which confuses objtool. This change, > > based on Josh's initial patch [1], goes through the list of relocations > > and replaces jump table symbols with the actual function symbols. > > > > [1] https://lore.kernel.org/r/d743f4b36e120c06506567a9f87a062ae03da47f.1611263462.git.jpoimboe@redhat.com/ > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > --- > > tools/objtool/arch/x86/decode.c | 16 +++++++++ > > tools/objtool/elf.c | 51 ++++++++++++++++++++++++++++ > > tools/objtool/include/objtool/arch.h | 3 ++ > > tools/objtool/include/objtool/elf.h | 2 +- > > 4 files changed, 71 insertions(+), 1 deletion(-) > > > > diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c > > index bc821056aba9..318189c8065e 100644 > > --- a/tools/objtool/arch/x86/decode.c > > +++ b/tools/objtool/arch/x86/decode.c > > @@ -62,6 +62,22 @@ bool arch_callee_saved_reg(unsigned char reg) > > } > > } > > > > +unsigned long arch_cfi_section_reloc_offset(struct reloc *reloc) > > +{ > > + if (!reloc->addend) > > + return 0; > > + > > + if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32) > > + return reloc->addend + 4; > > + > > + return reloc->addend; > > +} > > + > > +unsigned long arch_cfi_jump_reloc_offset(unsigned long offset) > > +{ > > + return offset + 1; > > +} > > + > > unsigned long arch_dest_reloc_offset(int addend) > > { > > return addend + 4; > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c > > index 8676c7598728..05a5f51aad2c 100644 > > --- a/tools/objtool/elf.c > > +++ b/tools/objtool/elf.c > > @@ -18,6 +18,7 @@ > > #include <errno.h> > > #include <objtool/builtin.h> > > > > +#include <objtool/arch.h> > > #include <objtool/elf.h> > > #include <objtool/warn.h> > > > > @@ -291,6 +292,10 @@ static int read_sections(struct elf *elf) > > if (sec->sh.sh_flags & SHF_EXECINSTR) > > elf->text_size += sec->len; > > > > + /* Detect -fsanitize=cfi jump table sections */ > > + if (!strncmp(sec->name, ".text..L.cfi.jumptable", 22)) > > + sec->cfi_jt = true; > > + > > list_add_tail(&sec->list, &elf->sections); > > elf_hash_add(section, &sec->hash, sec->idx); > > elf_hash_add(section_name, &sec->name_hash, str_hash(sec->name)); > > @@ -576,6 +581,49 @@ static int read_rela_reloc(struct section *sec, int i, struct reloc *reloc, unsi > > return 0; > > } > > > > +/* > > + * CONFIG_CFI_CLANG replaces function relocations to refer to an intermediate > > + * jump table. Undo the conversion so objtool can make sense of things. > > + */ > > +static int fix_cfi_relocs(const struct elf *elf) > > +{ > > + struct section *sec; > > + struct reloc *reloc; > > + > > + list_for_each_entry(sec, &elf->sections, list) { > > + list_for_each_entry(reloc, &sec->reloc_list, list) { > > + struct reloc *cfi_reloc; > > + unsigned long offset; > > + > > + if (!reloc->sym->sec->cfi_jt) > > + continue; > > + > > + if (reloc->sym->type == STT_SECTION) > > + offset = arch_cfi_section_reloc_offset(reloc); > > + else > > + offset = reloc->sym->offset; > > + > > + /* > > + * The jump table immediately jumps to the actual function, > > + * so look up the relocation there. > > + */ > > + offset = arch_cfi_jump_reloc_offset(offset); > > Sorry, this comment is curious to me, it looks like we jump to the > offset+1, not directly to the actual function? Perhaps a comment > above arch_cfi_jump_reloc_offset() and/or amending this comment might > make it clearer? Sorry if this is obvious to others? Perhaps comments > can be cleaned up in a follow up, if this is not a bug? It looks like my response was sent only to Nick, so to summarize the brief off-list discussion: arch_cfi_jump_reloc_offset() returns the offset to a relocation when given the address of a jmp instruction in the CFI jump table. Here's an example: Disassembly of section .text..L.cfi.jumptable: 0000000000000000 <_printk.cfi_jt>: 0: e9 00 00 00 00 jmp 0x5 <_printk.cfi_jt+0x5> 0000000000000001: R_X86_64_PLT32 _printk-0x4 5: cc int3 6: cc int3 7: cc int3 We look at the relocation in the jump table to figure out the actual target function, in this case, _printk. Alternatively, we could look at the jump table symbol instead (i.e., _printk.cfi_jt) and use the name to figure out the target. Unfortunately, LLVM doesn't generate symbols for all the jump table entries, so we can't rely on them in objtool. What comes to the magic offset value, it's one because the first byte of the jmp instruction is the opcode and the relocation only applies to the rest of the instruction. I'll add a comment to arch_cfi_jump_reloc_offset() in v4 to clarify this. Sami
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index bc821056aba9..318189c8065e 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -62,6 +62,22 @@ bool arch_callee_saved_reg(unsigned char reg) } } +unsigned long arch_cfi_section_reloc_offset(struct reloc *reloc) +{ + if (!reloc->addend) + return 0; + + if (reloc->type == R_X86_64_PC32 || reloc->type == R_X86_64_PLT32) + return reloc->addend + 4; + + return reloc->addend; +} + +unsigned long arch_cfi_jump_reloc_offset(unsigned long offset) +{ + return offset + 1; +} + unsigned long arch_dest_reloc_offset(int addend) { return addend + 4; diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 8676c7598728..05a5f51aad2c 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -18,6 +18,7 @@ #include <errno.h> #include <objtool/builtin.h> +#include <objtool/arch.h> #include <objtool/elf.h> #include <objtool/warn.h> @@ -291,6 +292,10 @@ static int read_sections(struct elf *elf) if (sec->sh.sh_flags & SHF_EXECINSTR) elf->text_size += sec->len; + /* Detect -fsanitize=cfi jump table sections */ + if (!strncmp(sec->name, ".text..L.cfi.jumptable", 22)) + sec->cfi_jt = true; + list_add_tail(&sec->list, &elf->sections); elf_hash_add(section, &sec->hash, sec->idx); elf_hash_add(section_name, &sec->name_hash, str_hash(sec->name)); @@ -576,6 +581,49 @@ static int read_rela_reloc(struct section *sec, int i, struct reloc *reloc, unsi return 0; } +/* + * CONFIG_CFI_CLANG replaces function relocations to refer to an intermediate + * jump table. Undo the conversion so objtool can make sense of things. + */ +static int fix_cfi_relocs(const struct elf *elf) +{ + struct section *sec; + struct reloc *reloc; + + list_for_each_entry(sec, &elf->sections, list) { + list_for_each_entry(reloc, &sec->reloc_list, list) { + struct reloc *cfi_reloc; + unsigned long offset; + + if (!reloc->sym->sec->cfi_jt) + continue; + + if (reloc->sym->type == STT_SECTION) + offset = arch_cfi_section_reloc_offset(reloc); + else + offset = reloc->sym->offset; + + /* + * The jump table immediately jumps to the actual function, + * so look up the relocation there. + */ + offset = arch_cfi_jump_reloc_offset(offset); + cfi_reloc = find_reloc_by_dest(elf, reloc->sym->sec, offset); + + if (!cfi_reloc || !cfi_reloc->sym) { + WARN("can't find a CFI jump table relocation at %s+0x%lx", + reloc->sym->sec->name, offset); + return -1; + } + + reloc->sym = cfi_reloc->sym; + reloc->addend = 0; + } + } + + return 0; +} + static int read_relocs(struct elf *elf) { struct section *sec; @@ -639,6 +687,9 @@ static int read_relocs(struct elf *elf) tot_reloc += nr_reloc; } + if (fix_cfi_relocs(elf)) + return -1; + if (stats) { printf("max_reloc: %lu\n", max_reloc); printf("tot_reloc: %lu\n", tot_reloc); diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h index 062bb6e9b865..2205b2b08268 100644 --- a/tools/objtool/include/objtool/arch.h +++ b/tools/objtool/include/objtool/arch.h @@ -81,6 +81,9 @@ unsigned long arch_jump_destination(struct instruction *insn); unsigned long arch_dest_reloc_offset(int addend); +unsigned long arch_cfi_section_reloc_offset(struct reloc *reloc); +unsigned long arch_cfi_jump_reloc_offset(unsigned long offset); + const char *arch_nop_insn(int len); int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg); diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index e34395047530..d9c1dacc6572 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -39,7 +39,7 @@ struct section { char *name; int idx; unsigned int len; - bool changed, text, rodata, noinstr; + bool changed, text, rodata, noinstr, cfi_jt; }; struct symbol {
With CONFIG_CFI_CLANG, the compiler replaces function references with references to the CFI jump table, which confuses objtool. This change, based on Josh's initial patch [1], goes through the list of relocations and replaces jump table symbols with the actual function symbols. [1] https://lore.kernel.org/r/d743f4b36e120c06506567a9f87a062ae03da47f.1611263462.git.jpoimboe@redhat.com/ Reported-by: Sedat Dilek <sedat.dilek@gmail.com> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- tools/objtool/arch/x86/decode.c | 16 +++++++++ tools/objtool/elf.c | 51 ++++++++++++++++++++++++++++ tools/objtool/include/objtool/arch.h | 3 ++ tools/objtool/include/objtool/elf.h | 2 +- 4 files changed, 71 insertions(+), 1 deletion(-)