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):
- [High] Lack of validation for `nr_domains` causes out-of-bounds reads and arbitrary frees during the cleanup path.
--

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

perf header: Do validation of perf.data HEADER_CPU_DOMAIN_INFO

This commit adds bounds checking for cpu and domain indices read from
a perf.data file's HEADER_CPU_DOMAIN_INFO section. It ensures these
values do not exceed nr_cpus_avail and max_sched_domains respectively.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- 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
>  		cd_map[cpu]->cpu = cpu;
>  
>  		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 (!cd_map[cpu]->domains)
>  			return -1;

Could there still be an issue with an unvalidated nr_domains value?

If a malformed file provides an nr_domains value much larger than
max_sched_domains, the loop below will eventually fail and return -1.

This triggers session teardown and calls free_cpu_domain_info(), which
iterates up to cd_map[i]->nr_domains and accesses cd_map[i]->domains[j].

Since the array size is only max_sched_domains, could this cause an
out-of-bounds read on the heap? If the out-of-bounds data is non-zero,
would it be treated as a pointer to struct domain_info and passed to
zfree(), causing arbitrary frees?

Also, if the domains array allocation fails here, cd_map[cpu]->domains is
NULL. Would the cleanup loop then blindly dereference a NULL pointer while
trying to access cd_map[i]->domains[j]?

>  
>  		for (j = 0; j < nr_domains; j++) {
>  			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;
> +			}

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