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:
- [Medium] Unbounded memory accesses during per-CPU metadata parsing and printing allow out-of-bounds reads on maliciously crafted perf.data files.
--

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

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

cs_etm__process_auxtrace_info() checks that header.size covers
event_header_size + INFO_HEADER_SIZE (16 bytes total), but then
accesses ptr[CS_PMU_TYPE_CPUS] at offset 24 from the start of the
event.  A crafted 16-byte auxtrace_info event passes the size check
but reads out-of-bounds.

Include CS_ETM_HEADER_SIZE in the minimum size check so that the
global header entries (version, pmu_type_cpus, snapshot) are
guaranteed to fit within the event.

> 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;

[ ... ]

>  	if (dump_trace) {
>  		num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
>  		cs_etm__print_auxtrace_info(ptr, num_cpu);
>  	}
>  
>  	return cs_etm__process_auxtrace_info_full(event, session);
>  }

[Severity: Medium]
This is a pre-existing issue, but does this still leave an out-of-bounds read
if the event size only covers the global header, and the parsed num_cpu value
requires more data?

Looking at the dump_trace path above, cs_etm__print_auxtrace_info() uses 
num_cpu as a loop boundary:

tools/perf/util/cs-etm-base.c:cs_etm__print_auxtrace_info() {
        ...
        for (i = CS_HEADER_VERSION_MAX; cpu < num; cpu++) {
                ...
}

Could this read and print memory out of bounds if the remaining event size
isn't checked to ensure it contains all the variable-length per-CPU 
parameters?

Additionally, in cs_etm__process_auxtrace_info_full(), num_cpu is checked 
against the total size:

tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
        ...
        if (num_cpu > priv_size / (int)sizeof(u64))
                return -EINVAL;
        ...
}

Does this check only guarantee a single 64-bit word per CPU? If 
cs_etm__create_meta_blk() later consumes multiple words per CPU and 
dynamically advances the stream offset based on untrusted payload counts like 
nr_in_params, could this also result in reading past the end of the event 
payload before the final size match validation catches it?

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