Re: [PATCH 4/5] perf header: Validate bitmap size before allocating in do_read_bitmap()

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Integer overflow in process_mem_topology() leading to heap buffer overflow
--

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

perf header: Validate bitmap size before allocating in do_read_bitmap()

This patch updates do_read_bitmap() to validate the u64 bit count read
from the file before allocating memory. It rejects sizes that exceed
INT_MAX or exceed the remaining section bounds, preventing potential
large allocations or truncation issues with bitmap_zalloc().

Link: https://lore.kernel.org/linux-perf-users/20260414224622.2AE69C19425@smtp.kernel.org/

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 696f19739074e..89697aec914c9 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -290,6 +290,11 @@ static int do_read_bitmap(struct feat_fd *ff, unsigned long **pset, u64 *psize)
>  	if (ret)
>  		return ret;
>  
> +	/* bitmap_zalloc() takes an int; reject u64 values that truncate. */
> +	if (size > INT_MAX ||
> +	    BITS_TO_U64(size) > (ff->size - ff->offset) / sizeof(u64))
> +		return -1;
> +
>  	set = bitmap_zalloc(size);
>  	if (!set)
>  		return -ENOMEM;

This isn't a bug introduced by this patch, but since the commit message notes
that do_read_bitmap() is used by process_mem_topology(), does the same missing
validation exist when allocating the nodes array in process_mem_topology()?

tools/perf/util/header.c:process_mem_topology() {
    ...
	if (do_read_u64(ff, &nr))
		return -1;

	nodes = zalloc(sizeof(*nodes) * nr);
	if (!nodes)
		return -1;

	for (i = 0; i < nr; i++) {
		struct memory_node n;
    ...
		nodes[i] = n;
	}
    ...
}

Because nr is an unvalidated 64-bit integer read from perf.data, could a
crafted value like 0x2000000000000000 cause the 64-bit multiplication
(24 * nr) to wrap around to 0 during the zalloc() call?

If zalloc(0) succeeds and allocates a minimum-sized heap chunk, the loop
could iterate and write beyond the allocated bounds when assigning
nodes[i] = n on subsequent iterations.

Could this overflow the nodes[] buffer?

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