Re: [PATCH 3/5] perf header: Sanity check HEADER_EVENT_DESC

sashiko-bot
2026-04-16T01:54:56Z

Sashiko AI review found 3 potential issue(s):
- [Critical] An integer overflow in `nre + 1` causes a 0-byte allocation, leading to a massive heap out-of-bounds write.
- [Critical] `perf_event__attr_swap()` reads and writes out-of-bounds on the heap due to an unvalidated internal `attr->size` field.
- [High] The expression `sz + sizeof(u32)` can overflow to zero on 32-bit architectures, causing a division by zero crash.
--

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

perf header: Sanity check HEADER_EVENT_DESC

This commit adds bounds checks to read_event_desc() to prevent large
allocations or excessive loop iterations by validating sizes against the
section size.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index f8acd06ae9235..696f19739074e 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
[ ... ]
> @@ -2101,6 +2101,13 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
> if (do_read_u32(ff, &sz))
> goto error;
>
> + /*
> + * The minimum section footprint per event is sz bytes for the attr
> + * plus a u32 for the id count, check that nre events fit.
> + */
> + if (sz == 0 || sz > ff->size || nre > (ff->size - ff->offset) / (sz + sizeof(u32)))
> + goto error;
> +

Can the expression sz + sizeof(u32) overflow to zero on 32-bit architectures?

Since sz is a u32 read directly from the file, if an attacker crafts a file
where sz is UINT32_MAX - 3 (0xFFFFFFFC), the addition wraps around to exactly 0
in 32-bit arithmetic. This would cause the bounds check to divide by zero.

The preceding sz > ff->size check might not prevent this because ff->size is
read from the file header and could be arbitrarily large.

> /* buffer to hold on file attr struct */
> buf = malloc(sz);
> if (!buf)
> goto error;
>
> /* the last event terminates with evsel->core.attr.size == 0: */
> events = calloc(nre + 1, sizeof(*events));

Could nre + 1 wrap around to 0 if nre is UINT32_MAX?

The new bounds check restricts nre based on ff->size, but if ff->size is
controlled by the file and set to UINT64_MAX, nre can still be 0xFFFFFFFF.

If nre + 1 wraps to 0, calloc returns a minimal valid 0-byte allocation. The
loop below would then write out of bounds on its first iteration:

for (i = 0, evsel = events; i < nre; evsel++, i++) {
evsel->core.idx = i;
...

> if (!events)
> goto error;
>
> msz = sizeof(evsel->core.attr);
> if (sz < msz)
> msz = sz;
>
> for (i = 0, evsel = events; i < nre; evsel++, i++) {
> evsel->core.idx = i;
>
> /*
> * must read entire on-file attr struct to
> * sync up with layout.
> */
> if (__do_read(ff, buf, sz))
> goto error;
>
> if (ff->ph->needs_swap)
> perf_event__attr_swap(buf);

Is it possible for perf_event__attr_swap() to read and write beyond the buf
allocation here?

The buf allocation is sized using the user-provided sz. When
perf_event__attr_swap(buf) executes, it byte-swaps the attr->size field inside
the buffer, and then uses the newly swapped attr->size to determine how many
subsequent fields to byte-swap.

If sz is small, but the embedded attr->size in the payload is large,
perf_event__attr_swap() will swap fields based on the larger internal size
instead of the allocated sz bytes, causing an out-of-bounds access.

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