Re: [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL checks on `etmq` and `etmq->decoder` lead to segmentation faults on malformed or out-of-bounds auxtrace queues.
--

commit 97b84b25fe94e957ad4cb91dac4d3e8d49fe5b1d
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf cs-etm: Bounds-check CPU in cs_etm__get_queue()

cs_etm__get_queue() indexes etm->queues.queue_array[cpu] without
validating that cpu is within nr_queues. When processing
AUX_OUTPUT_HW_ID events, the cpu value comes from untrusted perf.data
trace payload and flows through cs_etm__process_trace_id_v0_1() and
cs_etm__queue_aux_fragment() without bounds checking, allowing an
out-of-bounds read with a crafted file. Add a bounds check in
cs_etm__get_queue() and NULL checks in all callers.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index d121c8f22028d..5d0664ff73b79 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -318,6 +324,9 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
>  		int ret;
>  
>  		etmq = etm->queues.queue_array[i].priv;
> +		if (!etmq)
> +			continue;
> +

[Severity: High]
This is a pre-existing issue, but since we are adding NULL checks for the priv
field here because auxtrace_queues__grow() zero-initializes new entries, are
there other places that might still be vulnerable to this?

For example, in cs_etm__process_auxtrace_event(), we index into the queue array
and pass the priv field directly to cs_etm__dump_event():

	err = auxtrace_queues__add_event(&etm->queues, session,
					 event, data_offset, &buffer);
	if (err)
		return err;

	if (dump_trace)
		if (auxtrace_buffer__get_data(buffer, fd)) {
			cs_etm__dump_event(etm->queues.queue_array[idx].priv, buffer);
			auxtrace_buffer__put_data(buffer);
		}

Then cs_etm__dump_event() unconditionally dereferences etmq->decoder, which
causes a segmentation fault if etmq is NULL:

static void cs_etm__dump_event(struct cs_etm_queue *etmq,
			       struct auxtrace_buffer *buffer)
{
	...
	color_fprintf(stdout, color,
		     ". ... CoreSight %s Trace data: size %#zx bytes\n",
		     cs_etm_decoder__get_name(etmq->decoder), buffer->size);

Additionally, if the queue is empty during decoder creation, etmq->decoder can
be NULL. Later in cs_etm__decode_data_block(), etmq->decoder is passed without
a NULL check:

	ret = cs_etm_decoder__process_data_block(etmq->decoder,
						 etmq->offset,

Could this be triggered by a malformed or out-of-bounds auxtrace index,
resulting in a crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=5