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

Pre-existing issues:
- [High] The patch attempts to prevent TOCTOU re-reads of mmap'd fields using `READ_ONCE()`, but it fails to close the critical time window between payload size validation in `session.c` and usage in `cpumap.c`. An attacker can mutate fields like `nr` or `mask_nr` in a FUSE-backed file after they are bounds-checked in `session.c`, causing `cpumap.c` to snapshot and use unvalidated malicious bounds, leading to severe out-of-bounds memory reads.
- [High] On 32-bit big-endian architectures, `perf_record_cpu_map_data__read_one_mask()` incorrectly swaps the upper and lower 32 bits of a 64-bit mask, inverting the logical CPU IDs.
- [High] A double-read of mmap'd bitmasks in `cpu_map__from_mask` can result in partially initialized CPU maps with unsorted entries, breaking map invariants.
--

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

perf cpumap: Reject RANGE_CPUS with start_cpu > end_cpu

This commit addresses potential issues with malformed perf.data files
by rejecting inverted CPU ranges, clamping the any_cpu flag, and adding
READ_ONCE() snapshots to prevent TOCTOU vulnerabilities on mmap'd data.

> 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
[ ... ]
> @@ -40,15 +41,16 @@ bool perf_record_cpu_map_data__test_bit(int i,
>  
>  /* Read ith mask value from data into the given 64-bit sized bitmap */
>  static void perf_record_cpu_map_data__read_one_mask(const struct perf_record_cpu_map_data *data,
> -						    int i, unsigned long *bitmap)
> +						    int i, unsigned long *bitmap,
> +						    u16 long_size)
>  {
>  #if __SIZEOF_LONG__ == 8
[ ... ]
>  #else
> -	if (data->mask32_data.long_size == 4) {
> +	if (long_size == 4) {
>  		bitmap[0] = data->mask32_data.mask[i];
>  		bitmap[1] = 0;
>  	} else {
>  #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>  		bitmap[0] = (unsigned long)(data->mask64_data.mask[i] >> 32);
>  		bitmap[1] = (unsigned long)data->mask64_data.mask[i];

[Severity: High]
This is a pre-existing issue, but does this mapping reverse the logical CPU
IDs on 32-bit big-endian systems?

The native u64 mask stores CPU k at bit k. When mapping this into the Linux
bitops unsigned long array on 32-bit machines, for_each_set_bit() evaluates
bit indices based strictly on array position. This means bitmap[0] logically
needs to contain bits 0-31 and bitmap[1] needs to contain bits 32-63,
regardless of host endianness.

By placing the upper 32 bits into bitmap[0] and the lower 32 bits into
bitmap[1], are we mapping CPUs 0-31 to bits 32-63, and CPUs 32-63 to
bits 0-31?

>  #else
>  		bitmap[0] = (unsigned long)data->mask64_data.mask[i];
>  		bitmap[1] = (unsigned long)(data->mask64_data.mask[i] >> 32);
>  #endif
>  	}
>  #endif
>  }
>  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);
>  	struct perf_cpu_map *map;

[Severity: High]
This isn't a bug introduced by this patch, but does this snapshot actually
close the TOCTOU window?

If the bounds validation against the event payload size occurs earlier in
perf_session__process_user_event() without snapshotting the value, couldn't
an attacker supply a small, valid nr value to pass the check there, and
then mutate the FUSE-backed memory to a large value (like 65535) before
we reach this READ_ONCE()?

If so, wouldn't this late snapshot just capture the unvalidated malicious
value, resulting in a massive allocation and out-of-bounds reads from
the perf.data mapping?

>  
> -	map = perf_cpu_map__empty_new(data->cpus_data.nr);
> +	map = perf_cpu_map__empty_new(nr);
>  	if (!map)
>  		return NULL;
[ ... ]
> @@ -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)
>  {
>  	DECLARE_BITMAP(local_copy, 64);
> -	int weight = 0, mask_nr = data->mask32_data.nr;
> +	int weight = 0, mask_nr;
[ ... ]
>  	for (int i = 0; i < mask_nr; i++) {
> -		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);
>  	}
>  
> @@ -106,11 +121,14 @@ static struct perf_cpu_map *cpu_map__from_mask(const struct perf_record_cpu_map_
>  		return NULL;
>  
>  	for (int i = 0, j = 0; i < mask_nr; i++) {
> -		int cpus_per_i = (i * data->mask32_data.long_size  * BITS_PER_BYTE);
> +		int cpus_per_i = (i * long_size * BITS_PER_BYTE);
>  		int cpu;
>  
> -		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);
>  		for_each_set_bit(cpu, local_copy, 64) {
> +			/* Guard against more set bits than the first pass counted */
> +			if (j >= weight)
> +				break;

[Severity: High]
This is a pre-existing issue, but can the double-read of the bitmask
result in an incorrectly sorted map?

The code reads the mask array twice: first to calculate the weight, and
second to populate the map. Since perf_record_cpu_map_data__read_one_mask()
reads directly from mmap'd memory without READ_ONCE(), the underlying data
could mutate between passes.

If the data changes so that fewer bits are set during the second pass,
the map is only partially populated. Because perf_cpu_map__empty_new()
initializes all slots to -1 (representing 'any CPU'), wouldn't those -1
values remain at the end of the array?

Since downstream logic relies on strict monotonic sorting (e.g.
perf_cpu_map__max() assumes the last element is the maximum CPU), could
returning -1 here lead to 0-sized allocations later and subsequent
out-of-bounds writes?

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