Re: [PATCH 2/9] perf stat: Bounds-check CPU index in topology aggregation callbacks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incomplete bounds-check fix: Missing NULL pointer check before accessing `env->cpu` array in `perf_env__get_*_aggr_by_cpu` callbacks.
- [High] Out-of-bounds heap read due to unvalidated `sample->cpu` index in `perf_event__preprocess_sample()`.
--

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

perf stat: Bounds-check CPU index in topology aggregation callbacks

This commit updates six perf_env__get_*_aggr_by_cpu() functions to properly
bounds-check the CPU index against env->nr_cpus_avail. This prevents
out-of-bounds heap reads when processing untrusted perf.data files.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1638,7 +1638,8 @@ static struct aggr_cpu_id perf_env__get_socket_aggr_by_cpu(struct perf_cpu cpu,
>  	struct perf_env *env = data;
>  	struct aggr_cpu_id id = aggr_cpu_id__empty();
>  
> -	if (cpu.cpu != -1)
> +	/* env->cpu[] has env->nr_cpus_avail entries; reject untrusted indices */
> +	if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail)
>  		id.socket = env->cpu[cpu.cpu].socket_id;

[Severity: High]
This is a pre-existing issue, but is it possible for env->cpu to be NULL
here while env->nr_cpus_avail is greater than zero?

When parsing an older perf.data file, process_cpu_topology() can free
env->cpu while leaving env->nr_cpus_avail populated.

If this occurs, the bounds check cpu.cpu < env->nr_cpus_avail would pass,
but accessing env->cpu[cpu.cpu] would result in a NULL pointer dereference.
Does this code need an additional check to ensure env->cpu is not NULL?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while looking at CPU index
bounds checking, is there a similar missing upper-bound check in
perf_event__preprocess_sample()?

In tools/perf/util/event.c:perf_event__preprocess_sample(), the untrusted
sample->cpu value is checked for negative values, but does not appear to be
checked against env->nr_cpus_avail before accessing the env->cpu array:

    if (al->cpu >= 0)
        al->socket = env->cpu[al->cpu].socket_id;

Could a large positive value for al->cpu bypass this check and cause an
out-of-bounds heap read from the env->cpu array?

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