Re: [PATCH 6/8] perf header: Do validation of perf.data HEADER_CPU_DOMAIN_INFO

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] This problem wasn't introduced by this patch, but the patch fails to completely validate untrusted data from the `perf.data` file, leaving a critical out-of-bounds read and arbitrary free vulnerability. The bounds validation checks `cpu >= nra` and `domain >= max_sched_domains` but misses the validation of `nr_domains`. The variable `nr_domains` is read directly from the file and stored in `cd_map[cpu]->nr_domains`, while the `domains` array is allocated with a size of `max_sched_domains` (which is also read from the file). There is no check ensuring `nr_domains <= max_sched_domains`. If an attacker crafts a file where `nr_domains > max_sched_domains` and induces an early exit (e.g., by providing an invalid `domain` index, which the patch now explicitly catches and returns `-1`), the parsing function aborts and the cleanup function `free_cpu_domain_info()` is invoked. The cleanup loop iterates `j` from `0` up to `cd_map[cpu]->nr_domains - 1`, reading `cd_map[cpu]->domains[j]`. This reads out of bounds of the `domains` array, interpreting adjacent heap memory as `struct domain_info *` pointers. If these out-of-bounds values are non-NULL, the code calls `zfree()` on their fields and the pointer itself, resulting in a severe memory corruption vulnerability.
--

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

perf header: Do validation of perf.data HEADER_CPU_DOMAIN_INFO

This commit validates the cpu and domain values read from perf.data files
against nr_cpus_avail and max_sched_domains. The newly added validation
prevents using out-of-bounds indices to access the cd_map and domains
arrays.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index a18f216f77c27..4925e33778b91 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c

[ ... ]

> @@ -3736,6 +3741,12 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
>  			if (do_read_u32(ff, &domain))
>  				return -1;
>  
> +			if (domain >= max_sched_domains) {
> +				pr_err("Invalid HEADER_CPU_DOMAIN_INFO: domain %d >= max_sched_domains (%d)\n",
> +				       domain, max_sched_domains);
> +				return -1;
> +			}
> +

Does this code leave nr_domains unvalidated against max_sched_domains?

Earlier in process_cpu_domain_info(), nr_domains is read and assigned to
the map, but the domains array is allocated using max_sched_domains:

	if (do_read_u32(ff, &nr_domains))
		return -1;

	cd_map[cpu]->nr_domains = nr_domains;

	cd_map[cpu]->domains = zalloc(sizeof(*d_info) * max_sched_domains);

If a perf.data file contains a nr_domains value greater than
max_sched_domains, and an invalid domain index triggers the new return -1
path shown above, could this lead to an out-of-bounds read during cleanup?

If the cleanup function iterates up to the unvalidated nr_domains, it would
read past the end of the domains array. If these out-of-bounds values are
then freed, could this regression result in an arbitrary free?

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