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] Aggressive global clamping of `sample.cpu` to 0 destroys valid event data on systems with large CPU counts.
--

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

perf session: Bound nr_cpus_avail and validate sample CPU

This commit introduces a bounds check on sample.cpu before calling tool
callbacks to prevent out-of-bounds access in downstream consumers. The
validation caps the CPU limit at MAX_NR_CPUS and clamps any out-of-range
CPU values to 0.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index cb1d851686d9c..69eb404f805d8 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c

[ ... ]

> @@ -2099,14 +2099,100 @@ static int perf_session__deliver_event(struct perf_session *session,
> +		/*
> +		 * 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) {

[Severity: High]
Does this global clamping destroy valid event data on systems with large
CPU counts?

If a system has more CPUs than MAX_NR_CPUS, capping nr_cpus_avail here
causes legitimate samples originating from CPUs beyond the maximum to
be evaluated as out-of-bounds. This permanently overwrites their CPU
telemetry to 0 for all subsequent event consumers, even those that
dynamically allocate arrays and do not require this limit.

> +			/*
> +			 * 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;
> +		}

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