Re: [PATCH v3 19/21] objtool/klp: Introduce objtool for arm64

[sashiko-bot@kernel.org]

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 <jpoimboe@kernel.org>

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?

> +
> +	insn->len = 4;
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778642120.git.jpoimboe@kernel.org?part=19