Re: [PATCH 13/29] perf cpumap: Reject RANGE_CPUS with start_cpu > end_cpu

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch attempts to fix a TOCTOU vulnerability by using READ_ONCE() to snapshot array lengths (`nr` and `mask_nr`), but fails to validate these snapshotted values against the event payload size. This allows a malicious FUSE-backed file to bypass upstream validation and trigger severe out-of-bounds reads into memory-mapped data.
--

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

perf cpumap: Reject RANGE_CPUS with start_cpu > end_cpu

This commit adds boundary checks to cpu_map__from_range() to reject
inverted ranges and clamps any_cpu to a boolean. It also hardens
cpu_map__from_mask() to reject unsupported long_size values and snapshots
mmap'd fields using READ_ONCE() to prevent TOCTOU vulnerabilities.

> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 11922e1ded844..b1e5c29c6e3ec 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
[ ... ]
> @@ -64,24 +66,27 @@ static void perf_record_cpu_map_data__read_one_mask(const struct perf_record_cpu
>  }
>  static struct perf_cpu_map *cpu_map__from_entries(const struct perf_record_cpu_map_data *data)
>  {
> +	/* Snapshot nr — data is mmap'd and could change between reads */
> +	u16 nr = READ_ONCE(data->cpus_data.nr);

[Severity: High]
This is a pre-existing issue, but while we are snapshotting nr to prevent
time-of-check to time-of-use re-reads, do we also need to validate the
snapshotted value against the event payload size?

If the underlying file is on a FUSE mount, could an attacker pass the
initial upstream validation in session.c, and then maliciously increase
the value before this READ_ONCE() is executed?

Since the snapshotted nr is used as the loop bound below, could this lead
to severe out-of-bounds reads into memory-mapped data?

>  	struct perf_cpu_map *map;
>  
> -	map = perf_cpu_map__empty_new(data->cpus_data.nr);
> +	map = perf_cpu_map__empty_new(nr);
[ ... ]
> @@ -93,11 +98,21 @@ static struct perf_cpu_map *cpu_map__from_entries(const struct perf_record_cpu_m
>  static struct perf_cpu_map *cpu_map__from_mask(const struct perf_record_cpu_map_data *data)
>  {
[ ... ]
> +	mask_nr = READ_ONCE(data->mask32_data.nr);
>  
>  	for (int i = 0; i < mask_nr; i++) {

[Severity: High]
This isn't a bug introduced by this patch, but a similar question applies
here.

Is it possible for the snapshotted mask_nr to exceed the actual event size
if it gets modified after the initial event validation?

Does this need a check against the event payload limit before we iterate?

> -		perf_record_cpu_map_data__read_one_mask(data, i, local_copy);
> +		perf_record_cpu_map_data__read_one_mask(data, i, local_copy, long_size);
>  		weight += bitmap_weight(local_copy, 64);
>  	}

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