Messages in this thread |  | | Date | Sun, 27 Feb 2022 14:20:55 -0800 | From | Josh Poimboeuf <> | Subject | Re: [PATCH v2 34/39] objtool: Validate IBT assumptions |
| |
On Sun, Feb 27, 2022 at 06:00:03PM +0100, Peter Zijlstra wrote: > > > @@ -3101,6 +3164,17 @@ static int validate_branch(struct objtoo > > > > > > if (insn->hint) { > > > state.cfi = *insn->cfi; > > > + if (ibt) { > > > + struct symbol *sym; > > > + > > > + if (insn->cfi->type == UNWIND_HINT_TYPE_REGS_PARTIAL && > > > + (sym = find_symbol_by_offset(insn->sec, insn->offset)) && > > > + insn->type != INSN_ENDBR && !insn->noendbr) { > > > + WARN_FUNC("IRET_REGS hint without ENDBR: %s", > > > + insn->sec, insn->offset, > > > + sym->name); > > > + } > > > > No need to print sym->name here, WARN_FUNC() already does it? > > Almost; perhaps the change to make is to either introduce WARN_SYM or > make WARN_FUNC also print !STT_FUNC symbols ?
In the case of no function, WARN_FUNC() falls back to printing sec+off. Is that not good enough?
> > > +static void validate_ibt_insn(struct objtool_file *file, struct instruction *insn) > > > +{ > > > + struct reloc *reloc = insn_reloc(file, insn); > > > + struct instruction *target; > > > + > > > + for (;;) { > > > + if (!reloc) > > > + return; > > > + > > > + target = validate_ibt_reloc(file, reloc); > > > + if (target) > > > + validate_ibt_target(file, insn, target); > > > + > > > + reloc = find_reloc_by_dest_range(file->elf, insn->sec, reloc->offset + 1, > > > + (insn->offset + insn->len) - (reloc->offset + 1)); > > > + } > > > > I'm confused about what this loop is trying to do. Why would an > > instruction have more than one reloc? It at least needs a comment. > > Because there are some :/ 'mov' can have an immediate and a > displacement, both needing a relocation.
<boom> mind blown. How did I not know this?
-- Josh
|  |