Re: [PATCH 7/8] perf tools: Use calloc() where applicable

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Integer truncation in process_mem_topology() leads to an undersized allocation and a heap buffer overflow when parsing a crafted perf.data file.
- [High] Integer truncation in mem2node__init() via bitmap_weight() and test_bit() causes an undersized allocation and heap buffer overflow.
--

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

perf tools: Use calloc() where applicable

This commit replaces instances of zalloc with calloc across the perf tools
codebase to better handle array allocations. It also adjusts includes for
the standard library and kernel zalloc headers where dependencies changed.

> 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
[ ... ]
> @@ -3260,7 +3260,7 @@ static int process_mem_topology(struct feat_fd *ff,
>  	if (do_read_u64(ff, &nr))
>  		return -1;
>  
> -	nodes = zalloc(sizeof(*nodes) * nr);
> +	nodes = calloc(nr, sizeof(*nodes));

While not introduced by this patch, nr is read as a u64 from the file.
On 32-bit architectures, passing it to calloc, which takes size_t,
truncates it. For example, if nr is 0x100000002, calloc allocates space for
only 2 elements.

However, the subsequent loop for (i = 0; i < nr; i++) uses the full 64-bit
nr value. Could this result in a heap buffer overflow when reading the
entries if parsing a crafted perf.data file?

> diff --git a/tools/perf/util/mem2node.c b/tools/perf/util/mem2node.c
> --- a/tools/perf/util/mem2node.c
> +++ b/tools/perf/util/mem2node.c
[ ... ]
> @@ -59,7 +59,7 @@ int mem2node__init(struct mem2node *map, struct perf_env *env)
>  		max += bitmap_weight(n->set, n->size);
>  	}
>  
> -	entries = zalloc(sizeof(*entries) * max);
> +	entries = calloc(max, sizeof(*entries));

Similarly, in the preceding loop, n->size is a u64, but bitmap_weight
expects an unsigned int for the number of bits. If n->size exceeds 32 bits,
it is truncated, resulting in max being too small and an undersized
allocation here.

Later, the loop for (bit = 0; bit < n->size; bit++) iterates u64 times
and calls test_bit(bit, n->set). Since test_bit expects an int or unsigned
int, bit is also truncated. Will this repeatedly re-test the first 32 bits
and cause out-of-bounds writes into the entries array?

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