Re: [PATCH v3 19/21] objtool/klp: Introduce objtool for arm64
Josh Poimboeuf <[email protected]>
| Newsgroups | org.kernel.vger.live-patching,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <7owxhuwmxb2kbaioac5cm3qgqgqezcn6vgz34md47zuvm3ajjj@sgxpuzvuulmm> |
On Fri, May 15, 2026 at 02:08:45AM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The arm64 instruction decoder misclassifies trap instructions (e.g., `brk #0x800` used for `BUG()`) as `INSN_OTHER` instead of `INSN_BUG`. > -- > > commit dd63f0c03bab6a47f34cfcb310454460653638ed > Author: Josh Poimboeuf <[email protected]> > > objtool/klp: Introduce objtool for arm64 > > This commit adds basic support for arm64 in objtool, initially enabling > only the "objtool klp" subcommands. > > > diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c > > new file mode 100644 > > index 0000000000000..47658c76e1af0 > > --- /dev/null > > +++ b/tools/objtool/arch/arm64/decode.c > > [ ... ] > > > +int arch_decode_instruction(struct objtool_file *file, const struct section *sec, > > + unsigned long offset, unsigned int maxlen, > > + struct instruction *insn) > > +{ > > [ ... ] > > > + if (ins == 0xd503201f) { > > + /* NOP: static branch */ > > + insn->type = INSN_NOP; > > + } else if ((ins & 0xfc000000) == 0x14000000) { > > [ ... ] > > > + } else if ((ins & 0x7e000000) == 0x36000000) { > > + /* TBZ/TBNZ: intra-TU sibling call */ > > + insn->type = INSN_JUMP_CONDITIONAL; > > + insn->immediate = sign_extend64((ins >> 5) & 0x3fff, 13); > > + } else { > > + insn->type = INSN_OTHER; > > + } > > Does this code misclassify trap instructions? > > On arm64, BUG() is implemented using the brk instruction (such as brk #0x800). > Since the decoder doesn't appear to check for brk, it will fall through to the > else block and classify it as INSN_OTHER. > > If INSN_BUG is misclassified as a normal instruction, won't this lose dead-end > propagation and corrupt objtool's control-flow analysis? Nope, as the comment above this says: /* * These are the bare minimum needed for static branch detection and * checksum calculations. */ For arm64 we don't have any reason to care about INSN_BUG. -- Josh