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

[email protected]
Newsgroups org.kernel.vger.live-patching,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.