[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: [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: [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