[PATCHES 0/5 v2] More perf.data header validation

[PATCHES 0/5 v2] More perf.data header validation

[Arnaldo Carvalho de Melo]

Hi,

	This is picking up from what was reported in the previous
series, pre-existing lack of perf.data file validation, processing files
and buffers in header.c in a similar fashion.

	There is more to process in the trace data, but that is a
different can of worms that needs to be dealt with in a similar,
upcoming patch series,

	This is probably 7.2 material, but if feeling this can still
sneak into 7.1, feel free to do it :-)

	Now lets see what Sashiko discovers while I still don't have it
running locally right after Claude, before submitting it publicly, which
will soon happen :-)

- Arnaldo

v2: Addressed sashiko comments, adding a patch to the series.

Arnaldo Carvalho de Melo (5):
  perf header: Add section bounds checking to the fd read path
  perf header: Validate string length before allocating in do_read_string()
  perf header: Sanity check HEADER_EVENT_DESC
  perf header: Validate bitmap size before allocating in do_read_bitmap()
  perf header: Fix 32-bit incompatibility in bitmap serialization

 tools/perf/util/header.c | 57 +++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH 1/5] perf header: Add section bounds checking to the fd read path

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

__do_read_buf() validates reads against ff->size (section size), but
__do_read_fd() had no such check, so a malformed perf.data with an
understated section size could cause reads past the end of the current
section into the next section's data.

Add the bounds check in __do_read(), the common caller of both helpers,
so it is enforced uniformly for both the fd and buf paths.

This requires two supporting changes:

- perf_file_section__process(): initialize ff->offset to 0 so it
  tracks bytes consumed from the section start (consistent with the
  buf path), rather than the absolute file position.

- process_build_id(): use lseek(ff->fd, 0, SEEK_CUR) to discover
  the current file position instead of reading ff->offset.

Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index f30e48eb3fc32da2..13bbf8df15f66cab 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -213,23 +213,23 @@ static int __do_read_fd(struct feat_fd *ff, void *addr, ssize_t size)
 
 	if (ret != size)
 		return ret < 0 ? (int)ret : -1;
+	ff->offset += size;
 	return 0;
 }
 
 static int __do_read_buf(struct feat_fd *ff, void *addr, ssize_t size)
 {
-	if (size > (ssize_t)ff->size - ff->offset)
-		return -1;
-
 	memcpy(addr, ff->buf + ff->offset, size);
 	ff->offset += size;
 
 	return 0;
-
 }
 
 static int __do_read(struct feat_fd *ff, void *addr, ssize_t size)
 {
+	if (size > (ssize_t)ff->size - ff->offset)
+		return -1;
+
 	if (!ff->buf)
 		return __do_read_fd(ff, addr, size);
 	return __do_read_buf(ff, addr, size);
@@ -2713,7 +2713,12 @@ static int process_tracing_data(struct feat_fd *ff __maybe_unused, void *data __
 
 static int process_build_id(struct feat_fd *ff, void *data __maybe_unused)
 {
-	if (perf_header__read_build_ids(ff->ph, ff->fd, ff->offset, ff->size))
+	off_t offset = lseek(ff->fd, 0, SEEK_CUR);
+
+	if (offset == (off_t)-1)
+		return -1;
+
+	if (perf_header__read_build_ids(ff->ph, ff->fd, offset, ff->size))
 		pr_debug("Failed to read buildids, continuing...\n");
 	return 0;
 }
@@ -4687,7 +4692,7 @@ static int perf_file_section__process(struct perf_file_section *section,
 		.fd	= fd,
 		.ph	= ph,
 		.size	= section->size,
-		.offset	= section->offset,
+		.offset	= 0,
 	};
 
 	if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
-- 
2.53.0

[PATCH 2/5] perf header: Validate string length before allocating in do_read_string()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

do_read_string() reads a u32 length from the file and immediately
allocates that many bytes. A crafted perf.data could claim a huge
string length, triggering a large allocation that would only be freed
moments later when __do_read() rejects the read against the section
bounds.

Check len against the remaining section size before the malloc().

Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 13bbf8df15f66cab..c27ed90727ea6629 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -269,6 +269,9 @@ static char *do_read_string(struct feat_fd *ff)
 	if (do_read_u32(ff, &len))
 		return NULL;
 
+	if (len > ff->size - ff->offset)
+		return NULL;
+
 	buf = malloc(len);
 	if (!buf)
 		return NULL;
-- 
2.53.0

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

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

read_event_desc() reads nre (event count), sz (attr size), and nr
(IDs per event) from the file and uses them to control allocations
and loops without validating them against the section size.

A crafted perf.data could trigger large allocations or many loop
iterations before __do_read() eventually rejects the reads.

Add bounds checks:
- Reject sz == 0 or sz exceeding the section size.
- Check that nre events fit in the remaining section, using the
  minimum per-event footprint of sz + sizeof(u32).
- Check that nr IDs fit in the remaining section before allocating.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c27ed90727ea6629..3302748bac786fdf 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2141,6 +2141,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;
+
 	/* buffer to hold on file attr struct */
 	buf = malloc(sz);
 	if (!buf)
@@ -2186,6 +2193,9 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
 		if (!nr)
 			continue;
 
+		if (nr > (ff->size - ff->offset) / sizeof(*id))
+			goto error;
+
 		id = calloc(nr, sizeof(*id));
 		if (!id)
 			goto error;
-- 
2.53.0

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

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

do_read_bitmap() reads a u64 bit count from the file and passes it
to bitmap_zalloc() without checking it against the remaining section
size. A crafted perf.data could trigger a large allocation that would
only fail later when the per-element reads exceed section bounds.

Additionally, bitmap_zalloc() takes an int parameter, so a crafted
size with bits set above bit 31 (e.g. 0x100000040) would pass the
section bounds check but truncate when passed to bitmap_zalloc(),
allocating a much smaller buffer than the subsequent read loop
expects.

Reject size values that exceed INT_MAX, and check that the data
needed (BITS_TO_U64(size) u64 values) fits in the remaining section
before allocating.

Currently used by process_mem_topology() for HEADER_MEM_TOPOLOGY.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/linux-perf-users/20260414224622.2AE69C19425@smtp.kernel.org/
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 3302748bac786fdf..e1fed6f1c5e2fa4b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -300,6 +300,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;
-- 
2.53.0

[PATCH 5/5] perf header: Fix 32-bit incompatibility in bitmap serialization

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

do_write_bitmap() and do_read_bitmap() serialize bitmaps to/from
perf.data using u64 elements, but the in-memory representation uses
unsigned long, which is 4 bytes on 32-bit architectures and 8 bytes
on 64-bit.

Both functions cast the unsigned long * bitmap to u64 * and iterate
in 8-byte steps. When BITS_TO_LONGS(size) is odd on a 32-bit system
the bitmap occupies an odd number of 4-byte longs, but the loop
accesses it in 8-byte chunks, making the last chunk extend 4 bytes
past the allocation:

  Write side: reads 4 bytes of heap data beyond the bitmap and
  writes it into the perf.data file (heap info leak).

  Read side: writes 8 bytes into a 4-byte tail, corrupting the
  4 bytes following the allocation on the heap.

Fix the write side by using memcpy with the actual remaining byte
count instead of blindly casting to u64 *.  Fix the read side by
allocating in u64 units (calloc(BITS_TO_U64(size), sizeof(u64)))
instead of bitmap_zalloc(), which allocates in unsigned long units,
so the buffer is always large enough for the u64 read loop.

On 64-bit architectures sizeof(unsigned long) == sizeof(u64), so
the behavior is unchanged.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/linux-perf-users/20260414224622.2AE69C19425@smtp.kernel.org/
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e1fed6f1c5e2fa4b..a12f3f4ef0b38e8f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -158,15 +158,25 @@ int do_write(struct feat_fd *ff, const void *buf, size_t size)
 /* Return: 0 if succeeded, -ERR if failed. */
 static int do_write_bitmap(struct feat_fd *ff, unsigned long *set, u64 size)
 {
-	u64 *p = (u64 *) set;
+	size_t byte_size = BITS_TO_LONGS(size) * sizeof(unsigned long);
 	int i, ret;
 
 	ret = do_write(ff, &size, sizeof(size));
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * The on-disk format uses u64 elements, but the in-memory bitmap
+	 * uses unsigned long, which is only 4 bytes on 32-bit architectures.
+	 * Copy with bounded size so the last element doesn't read past the
+	 * bitmap allocation when BITS_TO_LONGS(size) is odd.
+	 */
 	for (i = 0; (u64) i < BITS_TO_U64(size); i++) {
-		ret = do_write(ff, p + i, sizeof(*p));
+		u64 val = 0;
+		size_t off = i * sizeof(val);
+
+		memcpy(&val, (char *)set + off, min(sizeof(val), byte_size - off));
+		ret = do_write(ff, &val, sizeof(val));
 		if (ret < 0)
 			return ret;
 	}
@@ -300,12 +310,18 @@ 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. */
+	/* Bitmap APIs use int for nbits; 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);
+	/*
+	 * bitmap_zalloc() allocates in unsigned long units, which are only
+	 * 4 bytes on 32-bit architectures. The read loop below casts the
+	 * buffer to u64 * and writes 8-byte elements, so allocate in u64
+	 * units to ensure the buffer is large enough.
+	 */
+	set = calloc(BITS_TO_U64(size), sizeof(u64));
 	if (!set)
 		return -ENOMEM;
 
-- 
2.53.0

Re: [PATCH 1/5] perf header: Add section bounds checking to the fd read path

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [Critical] The newly added bounds check in `__do_read()` is vulnerable to an integer bypass on 32-bit architectures, allowing a severe heap buffer overflow.
- [Critical] Stack buffer overflow in `perf_header__read_build_ids` and `perf_header__read_build_ids_abi_quirk` via missing size bounds checks.
- [High] Missing null-termination enforcement in `do_read_string` causes an out-of-bounds heap read and information disclosure.
- [Medium] Adding the section bounds check to `__do_read()` breaks `perf report --header` functionality because `perf_file_section__fprintf_info()` leaves `ff.size` uninitialized (zero).
--

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

perf header: Add section bounds checking to the fd read path

This commit adds a bounds check to __do_read() to prevent reading past the end
of a section when using the file descriptor read path. It centralizes the
check for both buffer and fd paths. It also updates perf_file_section__process()
and process_build_id() to support this change.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 9142a8ba40195..ca0c9e80345b2 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
[ ... ]
>  static int __do_read(struct feat_fd *ff, void *addr, ssize_t size)
>  {
> +	if (size > (ssize_t)ff->size - ff->offset)
> +		return -1;
> +

In perf_file_section__fprintf_info(), the struct feat_fd ff is
instantiated using a compound literal that only initializes .fd and .ph,
leaving .size and .offset as 0.

When feature printing callbacks (like print_event_desc()) call into this
path, will this bounds check evaluate to true (e.g., 4 > 0 - 0) and cause the
read to fail? Could this break perf report --header functionality?

Also, on 32-bit architectures, ssize_t is a 32-bit signed integer. If a
crafted perf.data file provides a large 32-bit length for a dynamic feature
(such as data_len in process_bpf_prog_info()), does the size parameter
implicitly convert to a negative ssize_t (e.g., 0xFFFFFFF0 becomes -16)?

If so, could this negative value bypass the bounds check and cause the
subsequent readn() call in __do_read_fd() to overwrite memory past the
dynamically allocated heap buffer?

[ ... ]
> @@ -2655,7 +2655,12 @@ static int process_tracing_data(struct feat_fd *ff, void *data)
>  
>  static int process_build_id(struct feat_fd *ff, void *data __maybe_unused)
>  {
> -	if (perf_header__read_build_ids(ff->ph, ff->fd, ff->offset, ff->size))
> +	off_t offset = lseek(ff->fd, 0, SEEK_CUR);
> +
> +	if (offset == (off_t)-1)
> +		return -1;
> +
> +	if (perf_header__read_build_ids(ff->ph, ff->fd, offset, ff->size))
>  		pr_debug("Failed to read buildids, continuing...\n");
>  	return 0;
>  }

While looking at the build ID processing updated here, does
perf_header__read_build_ids() safely handle crafted event sizes?

If a perf.data file provides an event where bev.header.size < sizeof(bev),
could the length calculation len = bev.header.size - sizeof(bev) underflow
to a large positive value?

If it exceeds sizeof(bev) + PATH_MAX, would the subsequent
readn(input, filename, len) call overflow the filename stack buffer?

Additionally, while reviewing the string reading operations in this file
for regressions, does do_read_string() enforce null-termination?

The function reads a length len from the file, allocates exactly len bytes,
and copies the data into the buffer. If a crafted perf.data file lacks a null
terminator for a string feature, could subsequent C string operations like
strdup or printf read past the allocated buffer and cause an out-of-bounds
heap read?

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

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

[sashiko-bot]

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

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

[sashiko-bot]

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

Re: [PATCHES 0/5 v2] More perf.data header validation

[James Clark]

On 16/04/2026 01:14, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> 	This is picking up from what was reported in the previous
> series, pre-existing lack of perf.data file validation, processing files
> and buffers in header.c in a similar fashion.
> 
> 	There is more to process in the trace data, but that is a
> different can of worms that needs to be dealt with in a similar,
> upcoming patch series,
> 
> 	This is probably 7.2 material, but if feeling this can still
> sneak into 7.1, feel free to do it :-)
> 
> 	Now lets see what Sashiko discovers while I still don't have it
> running locally right after Claude, before submitting it publicly, which
> will soon happen :-)
> 
> - Arnaldo
> 
> v2: Addressed sashiko comments, adding a patch to the series.
> 
> Arnaldo Carvalho de Melo (5):
>    perf header: Add section bounds checking to the fd read path
>    perf header: Validate string length before allocating in do_read_string()
>    perf header: Sanity check HEADER_EVENT_DESC
>    perf header: Validate bitmap size before allocating in do_read_bitmap()
>    perf header: Fix 32-bit incompatibility in bitmap serialization
> 
>   tools/perf/util/header.c | 57 +++++++++++++++++++++++++++++++++-------
>   1 file changed, 48 insertions(+), 9 deletions(-)
> 

Reviewed-by: James Clark <james.clark@linaro.org>

Re: [PATCH 1/5] perf header: Add section bounds checking to the fd read path

[Arnaldo Carvalho de Melo]

On Thu, Apr 16, 2026 at 12:59:15AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 4 potential issue(s):
> - [Critical] The newly added bounds check in `__do_read()` is vulnerable to an integer bypass on 32-bit architectures, allowing a severe heap buffer overflow.
> - [Critical] Stack buffer overflow in `perf_header__read_build_ids` and `perf_header__read_build_ids_abi_quirk` via missing size bounds checks.
> - [High] Missing null-termination enforcement in `do_read_string` causes an out-of-bounds heap read and information disclosure.
> - [Medium] Adding the section bounds check to `__do_read()` breaks `perf report --header` functionality because `perf_file_section__fprintf_info()` leaves `ff.size` uninitialized (zero).
> --
> 
> commit a967949f0160c2901e835f301c2e340a342b95db
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf header: Add section bounds checking to the fd read path
> 
> This commit adds a bounds check to __do_read() to prevent reading past the end
> of a section when using the file descriptor read path. It centralizes the
> check for both buffer and fd paths. It also updates perf_file_section__process()
> and process_build_id() to support this change.
> 
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index 9142a8ba40195..ca0c9e80345b2 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> [ ... ]
> >  static int __do_read(struct feat_fd *ff, void *addr, ssize_t size)
> >  {
> > +	if (size > (ssize_t)ff->size - ff->offset)
> > +		return -1;
> > +
> 
> In perf_file_section__fprintf_info(), the struct feat_fd ff is
> instantiated using a compound literal that only initializes .fd and .ph,
> leaving .size and .offset as 0.
> 
> When feature printing callbacks (like print_event_desc()) call into this
> path, will this bounds check evaluate to true (e.g., 4 > 0 - 0) and cause the
> read to fail? Could this break perf report --header functionality?

Will fix.
 
> Also, on 32-bit architectures, ssize_t is a 32-bit signed integer. If a
> crafted perf.data file provides a large 32-bit length for a dynamic feature
> (such as data_len in process_bpf_prog_info()), does the size parameter
> implicitly convert to a negative ssize_t (e.g., 0xFFFFFFF0 becomes -16)?

Will fix.
 
> If so, could this negative value bypass the bounds check and cause the
> subsequent readn() call in __do_read_fd() to overwrite memory past the
> dynamically allocated heap buffer?
> 
> [ ... ]
> > @@ -2655,7 +2655,12 @@ static int process_tracing_data(struct feat_fd *ff, void *data)
> >  
> >  static int process_build_id(struct feat_fd *ff, void *data __maybe_unused)
> >  {
> > -	if (perf_header__read_build_ids(ff->ph, ff->fd, ff->offset, ff->size))
> > +	off_t offset = lseek(ff->fd, 0, SEEK_CUR);
> > +
> > +	if (offset == (off_t)-1)
> > +		return -1;
> > +
> > +	if (perf_header__read_build_ids(ff->ph, ff->fd, offset, ff->size))
> >  		pr_debug("Failed to read buildids, continuing...\n");
> >  	return 0;
> >  }
> 
> While looking at the build ID processing updated here, does
> perf_header__read_build_ids() safely handle crafted event sizes?
> 
> If a perf.data file provides an event where bev.header.size < sizeof(bev),
> could the length calculation len = bev.header.size - sizeof(bev) underflow
> to a large positive value?
> 
> If it exceeds sizeof(bev) + PATH_MAX, would the subsequent
> readn(input, filename, len) call overflow the filename stack buffer?

will check
 
> Additionally, while reviewing the string reading operations in this file
> for regressions, does do_read_string() enforce null-termination?
> 
> The function reads a length len from the file, allocates exactly len bytes,
> and copies the data into the buffer. If a crafted perf.data file lacks a null
> terminator for a string feature, could subsequent C string operations like
> strdup or printf read past the allocated buffer and cause an out-of-bounds
> heap read?

I think Ian Rogers is working on this one.

- Arnaldo

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

[Arnaldo Carvalho de Melo]

On Thu, Apr 16, 2026 at 01:54:55AM +0000, sashiko-bot@kernel.org wrote:
> 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?

Will fix
 
> 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.

So this is valid but pre-existing, no? I'll cover this in an upcoming
patch series.

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

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

[Arnaldo Carvalho de Melo]

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

Re: [PATCHES 0/5 v2] More perf.data header validation

[Arnaldo Carvalho de Melo]

On Thu, Apr 16, 2026 at 02:17:37PM +0100, James Clark wrote:
> On 16/04/2026 01:14, Arnaldo Carvalho de Melo wrote:
> > Hi,
> > 
> > 	This is picking up from what was reported in the previous
> > series, pre-existing lack of perf.data file validation, processing files
> > and buffers in header.c in a similar fashion.
> > 
> > 	There is more to process in the trace data, but that is a
> > different can of worms that needs to be dealt with in a similar,
> > upcoming patch series,
> > 
> > 	This is probably 7.2 material, but if feeling this can still
> > sneak into 7.1, feel free to do it :-)
> > 
> > 	Now lets see what Sashiko discovers while I still don't have it
> > running locally right after Claude, before submitting it publicly, which
> > will soon happen :-)
> > 
> > - Arnaldo
> > 
> > v2: Addressed sashiko comments, adding a patch to the series.
> > 
> > Arnaldo Carvalho de Melo (5):
> >    perf header: Add section bounds checking to the fd read path
> >    perf header: Validate string length before allocating in do_read_string()
> >    perf header: Sanity check HEADER_EVENT_DESC
> >    perf header: Validate bitmap size before allocating in do_read_bitmap()
> >    perf header: Fix 32-bit incompatibility in bitmap serialization
> > 
> >   tools/perf/util/header.c | 57 +++++++++++++++++++++++++++++++++-------
> >   1 file changed, 48 insertions(+), 9 deletions(-)
> > 
> 
> Reviewed-by: James Clark <james.clark@linaro.org>

Thanks! I'm replying with a few new fixes and will tentatively keep your
Reviewed-by, please check when it get to the mailing list.

I also made sure it ran checkpatch and fixed the two minor issues it
noticed: order of tags and replacing a Link: after a Reported-by with a
Closes:, also reduced the length of a subject line.

- Arnaldo

Re: [PATCHES 0/5 v2] More perf.data header validation

[Namhyung Kim]

Hi guys,

On Thu, Apr 16, 2026 at 12:28:27PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Apr 16, 2026 at 02:17:37PM +0100, James Clark wrote:
> > On 16/04/2026 01:14, Arnaldo Carvalho de Melo wrote:
> > > Hi,
> > > 
> > > 	This is picking up from what was reported in the previous
> > > series, pre-existing lack of perf.data file validation, processing files
> > > and buffers in header.c in a similar fashion.
> > > 
> > > 	There is more to process in the trace data, but that is a
> > > different can of worms that needs to be dealt with in a similar,
> > > upcoming patch series,
> > > 
> > > 	This is probably 7.2 material, but if feeling this can still
> > > sneak into 7.1, feel free to do it :-)

Right, I don't want to add non-urgent changes to 7.1 at this point.
I'll work on finalizing the PR message.

Thanks,
Namhyung

Re: [PATCHES 0/5 v2] More perf.data header validation

[Arnaldo Carvalho de Melo]

On Thu, Apr 16, 2026 at 09:46:47AM -0700, Namhyung Kim wrote:
> On Thu, Apr 16, 2026 at 12:28:27PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Thu, Apr 16, 2026 at 02:17:37PM +0100, James Clark wrote:
> > > On 16/04/2026 01:14, Arnaldo Carvalho de Melo wrote:
> > > > 	This is probably 7.2 material, but if feeling this can still
> > > > sneak into 7.1, feel free to do it :-)
 
> Right, I don't want to add non-urgent changes to 7.1 at this point.
> I'll work on finalizing the PR message.

Agreed, I'm addressing some more Sashiko review comments and trying to
run it locally before sending a new version for this series.

- Arnaldo