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

Arnaldo Carvalho de Melo
2026-04-16T15:26:35Z

On Thu, Apr 16, 2026 at 02:25:38AM +0000, sashiko-bot@kernel.org wrote:
> 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);

This code now already has a bounds check and is using calloc:

if (do_read_u64(ff, &nr))
return -1;

if (nr > MAX_NUMA_NODES) {
pr_err("Invalid HEADER_MEM_TOPOLOGY: nr_nodes (%llu) > %u\n",
(unsigned long long)nr, MAX_NUMA_NODES);
return -1;
}

if (ff->size < 3 * sizeof(u64) + nr * 2 * sizeof(u64)) {
pr_err("Invalid HEADER_MEM_TOPOLOGY: section too small (%zu) for %llu nodes\n",
ff->size, (unsigned long long)nr);
return -1;
}

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

Probably you're working with upstream and the fixes are in
perf-tools-next/perf-tools-next.

> 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