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 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds checks when parsing variable-length per-CPU metadata in CS-ETM auxtrace_info allows out-of-bounds reads.
- [High] Missing size validations in auxtrace info processing for Intel PT, Intel BTS, and PowerPC VPADTL allow out-of-bounds memory reads from the event payload.
- [High] Insufficient bounds checking and untrusted input usage in Arm SPE auxtrace info processing lead to division by zero and out-of-bounds memory accesses.
--

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

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

This commit updates cs_etm__process_auxtrace_info() to include
CS_ETM_HEADER_SIZE in the minimum size check. This ensures that the global
header entries fit within the event, fixing an out-of-bounds read issue
with crafted auxtrace_info events.

> 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 the patch attempts to fix a
related out-of-bounds read by validating the global header size. Is the fix
incomplete?

The per-CPU metadata is still read dynamically without verifying that the
event payload (auxtrace_info->header.size) is large enough to contain it.

In cs_etm__process_auxtrace_info(), if dump_trace is true,
cs_etm__print_auxtrace_info() reads at least 7 64-bit words per CPU without
checking the buffer bounds, leading to an out-of-bounds read and exposing heap
memory contents to standard output:

tools/perf/util/cs-etm-base.c:cs_etm__print_cpu_metadata_v1() {
    ...
    total_params = val[i + CS_ETM_NR_TRC_PARAMS] + CS_ETM_COMMON_BLK_MAX_V1;
    ...
}

Similarly, cs_etm__process_auxtrace_info_full() validates that there is at
least 1 word per CPU, but cs_etm__create_meta_blk() iterates and reads multiple
words per CPU based on CS_ETM_NR_TRC_PARAMS, causing out-of-bounds heap reads:

tools/perf/util/cs-etm.c:cs_etm__create_meta_blk() {
    ...
    for (k = CS_ETM_MAGIC; k < nr_out_params; k++)
        metadata[k] = buff_in[i + k];
    ...
}

And later validates the total number of words read only after the out-of-bounds
memory accesses have already occurred:

tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
    ...
    if (i * 8 != priv_size) {
    ...
}

Could a crafted trace file with insufficient per-CPU metadata size crash the
perf tool or leak memory?

[Severity: High]
This is a pre-existing issue, but it exists in similar auxtrace decoders for
Intel PT, Intel BTS, and PowerPC VPADTL. Do these decoders also miss size
validations, allowing out-of-bounds memory reads from the event payload?

In intel_pt_process_auxtrace_info(), min_sz is calculated as sizeof(u64) *
INTEL_PT_PER_CPU_MMAPS (which is sizeof(u64) * 9). This guarantees the payload
has space for indices 0 to 8. The code then reads index 9, which accesses 8
bytes strictly beyond the validated boundary if the size is exactly min_sz:

tools/perf/util/intel-pt.c:intel_pt_process_auxtrace_info() {
    ...
    pt->per_cpu_mmaps = auxtrace_info->priv[INTEL_PT_PER_CPU_MMAPS];
    ...
}

The exact same off-by-one enum indexing pattern causes out-of-bounds reads in
intel_bts_process_auxtrace_info(), which validates up to
INTEL_BTS_SNAPSHOT_MODE but accesses index 5:

tools/perf/util/intel-bts.c:intel_bts_process_auxtrace_info() {
    ...
    bts->snapshot_mode = auxtrace_info->priv[INTEL_BTS_SNAPSHOT_MODE];
    ...
}

And in powerpc_vpadtl_process_auxtrace_info(), which validates up to
POWERPC_VPADTL_TYPE (0 bytes) but reads index 0:

tools/perf/util/powerpc-vpadtl.c:powerpc_vpadtl_process_auxtrace_info() {
    ...
    vpa->pmu_type = auxtrace_info->priv[POWERPC_VPADTL_TYPE];
    ...
}

Can parsing a crafted perf.data file trigger out-of-bounds heap reads and
potential segmentation faults in these decoders?

[Severity: High]
This is a pre-existing issue, but it also exists in the Arm SPE auxtrace
decoder. Are there insufficient bounds checks and untrusted input usage in
arm_spe_process_auxtrace_info() that lead to division by zero and
out-of-bounds memory accesses?

The event size is only verified to be at least ARM_SPE_AUXTRACE_V1_PRIV_SIZE
(16 bytes). If the size is 17 bytes, it skips the V1 branch and reads index 3
(offset 24) from the payload, causing an immediate out-of-bounds read:

tools/perf/util/arm-spe.c:arm_spe__alloc_metadata() {
    ...
    *nr_cpu = ptr[ARM_SPE_CPUS_NUM];
    ...
}

Furthermore, *nr_cpu is taken directly from the untrusted payload without
checking for zero, and allows unsigned wraparound if hdr_sz is larger than
metadata_size:

tools/perf/util/arm-spe.c:arm_spe__alloc_metadata() {
    ...
    per_cpu_sz = (metadata_size - (hdr_sz * sizeof(u64))) / (*nr_cpu);
    ...
}

This could result in a huge per_cpu_sz value passed to memcpy(), leading to a
massive out-of-bounds heap read and segmentation fault:

tools/perf/util/arm-spe.c:arm_spe__alloc_per_cpu_metadata() {
    ...
    memcpy(metadata, buf, per_cpu_size);
    ...
}

Can a crafted perf.data file with nr_cpu == 0 cause a SIGFPE and crash the
perf tool?

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