Re: [PATCH 26/29] perf session: Bound nr_cpus_avail and validate sample CPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Globally mutating `sample.cpu` to 0 for CPUs exceeding `MAX_NR_CPUS` corrupts valid data for tools that correctly support large CPU counts.
--

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

perf session: Bound nr_cpus_avail and validate sample CPU
    
This commit adds a bounds check for the sample CPU index in the core event
delivery loop to protect downstream tools with fixed-size arrays. It bounds
the available CPU count to MAX_NR_CPUS and clamps any out-of-bounds
sample.cpu value to 0.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 9271885e3920f..6de665d3c9054 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c

[ ... ]

> +		/*
> +		 * Cap at MAX_NR_CPUS for the bounds check — downstream
> +		 * consumers use fixed-size arrays of that size.  Keep
> +		 * the true nr_cpus_avail in env for header parsing
> +		 * (e.g. process_cpu_topology) which needs the real count.
> +		 */
> +		if (nr_cpus_avail > MAX_NR_CPUS)
> +			nr_cpus_avail = MAX_NR_CPUS;
> +		if (sample.cpu >= (u32)nr_cpus_avail &&
> +		    sample.cpu != (u32)-1) {
> +			/*
> +			 * Warn rather than abort: synthesized events
> +			 * (MMAP, COMM) lack sample_id_all data, so
> +			 * parse_id_sample reads garbage from the event
> +			 * payload.  Clamping to 0 protects downstream
> +			 * array indexing while keeping the session alive.
> +			 *
> +			 * Preserve (u32)-1: perf script and perf inject
> +			 * use it as a sentinel for "CPU not applicable."
> +			 * Downstream array users (timechart, kwork) have
> +			 * their own per-callback bounds checks.
> +			 */
> +			pr_warning_once("WARNING: sample CPU %u >= nr_cpus_avail %u, clamping to 0\n",
> +					sample.cpu, nr_cpus_avail);
> +			sample.cpu = 0;

[Severity: High]
Does this global mutation of sample.cpu corrupt valid data for tools that
correctly support large CPU counts?

If a system legitimately has more than MAX_NR_CPUS (4096) CPUs, clamping
nr_cpus_avail to MAX_NR_CPUS and then reassigning sample.cpu to 0 will
attribute all samples from CPU 4096 and above to CPU 0.

Downstream tools like perf script and perf report handle CPU indices
dynamically and fully support greater than 4096 CPUs.

Would it be safer to push this fixed-size array bounds checking down into
the specific tools (like timechart and kwork) rather than altering the
shared sample structure globally in the core event delivery loop?

> +		}
> +	}
>  
>  	ret = auxtrace__process_event(session, event, &sample, tool);

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