Re: [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The commit fixes an out-of-bounds read for the global header but leaves identical vulnerabilities in the per-CPU metadata printing and parsing paths. A crafted event with an untrusted `num_cpu` or dynamically read parameter offsets can trigger out-of-bounds memory reads and information leaks.
--

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

perf cs-etm: Require full global header in auxtrace_info size check

This commit includes CS_ETM_HEADER_SIZE in the minimum size check for
auxtrace_info events. This ensures that the global header entries fit within
the event, fixing an out-of-bounds read.

> diff --git a/tools/perf/util/cs-etm-base.c b/tools/perf/util/cs-etm-base.c
> index 4abe416e3febd..aebef71d3a0a1 100644
> --- a/tools/perf/util/cs-etm-base.c
> +++ b/tools/perf/util/cs-etm-base.c
> @@ -170,7 +170,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  	u64 *ptr = NULL;
>  	u64 hdr_version;
>  
> -	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE))
> +	/* Ensure priv[] is large enough for the global header entries */
> +	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE +
> +					  CS_ETM_HEADER_SIZE))
>  		return -EINVAL;
>  
>  	/* First the global part */

[Severity: High]
This isn't a bug introduced by this patch, but while this size check fixes
the out-of-bounds read for the global header, does this code leave similar
out-of-bounds reads unaddressed in the per-CPU metadata processing?

When dump_trace is true, cs_etm__process_auxtrace_info() extracts num_cpu 
and passes it to cs_etm__print_auxtrace_info():

    if (dump_trace) {
        num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
        cs_etm__print_auxtrace_info(ptr, num_cpu);
    }

Can a crafted perf.data file provide an untrusted num_cpu without it being
bounded against the remaining event size?

In cs_etm__print_auxtrace_info(), this loops num_cpu times and iteratively
calls metadata printing functions:

tools/perf/util/cs-etm-base.c:cs_etm__print_auxtrace_info() {
    ...
    for (i = CS_HEADER_VERSION_MAX; cpu < num; cpu++) {
        if (version == 0)
            err = cs_etm__print_cpu_metadata_v0(val, &i);
    ...
}

Could this read unbounded parameters and potentially leak heap memory to
stdout?

Additionally, in the parsing loop, cs_etm__process_auxtrace_info_full() 
calls cs_etm__create_meta_blk():

tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
    ...
    for (int j = 0; j < num_cpu; j++) {
        if (ptr[i] == __perf_cs_etmv3_magic) {
            metadata[j] = cs_etm__create_meta_blk(...);
    ...
}

Inside cs_etm__create_meta_blk(), parameter counts are read to advance the
parsing offset:

    nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS];
    ...
    i += nr_in_params + nr_cmn_params;
    *buff_in_offset = i;

Since the offset i is advanced dynamically without validating against the
remaining event capacity, could the next iteration of the loop in
cs_etm__process_auxtrace_info_full() read ptr[i] out-of-bounds before the
post-loop size validation is reached?

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