Re: [PATCH v9 8/9] perf cs-etm: Synthesize callchains for instruction samples

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Prematurely freeing `tidq->frontend_thread` on the error path in `cs_etm__context()` leads to a fatal NULL pointer dereference.
--

commit beaeb914d5e99fcdf91b04714fe39931a7466896
Author: Leo Yan <leo.yan@linux.dev>

perf cs-etm: Synthesize callchains for instruction samples

This commit synthesizes callchains for CS ETM instruction samples. It allocates
a callchain buffer per queue, uses thread_stack__sample() for instruction
samples, and introduces cs_etm__context() to handle context packets.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index e18d2dec79431..34489476681b8 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
[ ... ]
> @@ -1904,6 +1941,34 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
>  	return 0;
>  }
>  
> +static int cs_etm__context(struct cs_etm_queue *etmq,
> +			   struct cs_etm_traceid_queue *tidq)
> +{
> +	ocsd_ex_level el = tidq->packet->el;
> +	struct machine *machine;
> +	int ret;
> +
> +	machine = cs_etm__get_machine(etmq, el);
> +	if (!machine) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	tidq->kernel_start = machine__kernel_start(machine);
> +
> +	ret = cs_etm__etmq_update_thread(etmq, el, tidq->packet->tid,
> +					 &tidq->frontend_thread);
> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	thread__zput(tidq->frontend_thread);

[Severity: Medium]
Will this error path result in a NULL pointer dereference?

If cs_etm__get_machine() or cs_etm__etmq_update_thread() fails, thread__zput()
is called which drops the reference and sets tidq->frontend_thread to NULL.

However, the calling loops, such as cs_etm__run_per_cpu_timeless_decoder()
which calls cs_etm__process_traceid_queue(), can ignore the error and continue
processing trace buffer packets.

When a subsequent packet like CS_ETM_RANGE is processed, it calls
cs_etm__synth_instruction_sample() which unconditionally dereferences the NULL
thread pointer:

cs_etm__synth_instruction_sample() {
    ...
    sample.pid = thread__pid(tidq->frontend_thread);
    ...
}

Should the frontend_thread pointer be preserved on failure, or should the
caller be updated to abort packet processing when an error occurs?

> +	tidq->kernel_start = 0;
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-b4-arm_cs_callchain_support_v1-v9-0-f8fad931c413@arm.com?part=8