Re: [PATCH v2 05/14] perf cs-etm: Use the recorded instruction size for A32 and A64

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf cs-etm: Missing bounds check on instruction size in cs_etm__copy_insn()
--

commit 8ca3f124dea0462e31844bee55fa404297bf8a53
Author: Leo Yan <leo.yan@arm.com>

perf cs-etm: Use the recorded instruction size for A32 and A64

Use packet->last_instr_size in cs_etm__instr_size() instead of a
hard-coded value, allowing the decoder to supply the instruction size.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 2d1ab34f7b6b5..0163d93ac2d53 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1358,8 +1358,7 @@ static inline int cs_etm__instr_size(struct cs_etm_queue *etmq,
>  	if (packet->isa == CS_ETM_ISA_T32)
>  		return cs_etm__t32_instr_size(etmq, tidq, packet, addr);
>  
> -	/* Otherwise, 4-byte instruction size for A32/A64 */
> -	return 4;
> +	return packet->last_instr_size;

[Severity: High]
Can an untrusted decoder-supplied size cause this regression to overflow
sample->insn[]?

When cs_etm__instr_size() returns packet->last_instr_size without validation,
it is assigned to sample->insn_len in cs_etm__copy_insn().

This unbounded size is then passed to cs_etm__frontend_mem_access() as the
read length, which writes into the sample->insn[] array:

tools/perf/util/cs-etm.c:cs_etm__copy_insn() {
...
	sample->insn_len = cs_etm__instr_size(etmq, tidq, packet, sample->ip);

	cs_etm__frontend_mem_access(etmq, tidq, packet, sample->ip,
				    sample->insn_len, (void *)sample->insn);
...
}

Because sample->insn[] is defined as a fixed size array (char insn[MAX_INSN])
within struct perf_sample, processing an untrusted or maliciously crafted
perf.data file where packet->last_instr_size exceeds MAX_INSN might cause an
out-of-bounds write on the stack.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-perf_cs_etm_fix_non_taken-v2-0-6ab8c07a5455@arm.com?part=5