[PATCH v4 0/5] perf trace-event: Fix overflow, loop and cleanup bugs

[PATCH v4 0/5] perf trace-event: Fix overflow, loop and cleanup bugs

[Tanushree Shah]

This series fixes five security issues in trace-event-read.c and
trace-event.c:
1. Stack buffer overflow in read_string() when a string exceeds
   BUFSIZ, due to a missing bounds check.
2. Integer truncation when passing 64-bit sizes into do_read() and
   skip(), which use 'int' parameters, causing uninitialized memory
   to be dumped and parsers to read out of bounds.
3. Double free / use-after-free in trace_event__cleanup(): it frees
   t->pevent but never clears the pointer, so calling it twice on
   the same trace_event touches already-freed memory. Also fixes a
   related leak in trace_event__init(), which overwrites
   t->pevent/t->plugin_list without releasing any existing handle
   if called more than once on the same struct.
4. Heap buffer overflow in read_ftrace_printk() and
   read_saved_cmdline(): size + 1 can overflow to 0 in malloc(),
   allocating a tiny buffer while a huge read is still attempted
   into it.
5. Infinite loop in skip(): it does not check do_read()'s return
   value, so a crafted size can spin the loop indefinitely.

Patches 4 and 5 are latent bugs that patch 2's type fix exposes by
removing accidental truncation that had been masking them.

Tanushree Shah (5):
  perf trace-event: Fix buffer overflow in read_string()
  perf trace-event: Fix integer truncation in do_read() and skip()
  perf trace-event: Avoid double free and leak in
    trace_event__cleanup()/trace_event__init()
  perf trace-event: Fix heap overflows in
    read_ftrace_printk()/read_saved_cmdline()
  perf trace-event: Fix infinite loop in skip()

 tools/perf/util/trace-event-read.c | 54 ++++++++++++++++++++----------
 tools/perf/util/trace-event.c      | 11 +++++-
 2 files changed, 47 insertions(+), 18 deletions(-)

---
Changes in v4:
- Fixed a leak in trace_event__init(): repeated calls overwrite
  t->pevent/t->plugin_list without releasing the existing handle.
- Fixed the same size + 1 overflow in read_saved_cmdline() as
  read_ftrace_printk() (size == ULLONG_MAX wraps to 0 in malloc()).
- Added <limits.h> for the UINT_MAX/ULLONG_MAX checks.

Changes in v3:
- Added new patch to fix double free in trace_event__cleanup().
- Added new patch to fix heap buffer overflow in read_ftrace_printk().
- Added new patch to fix infinite loop in skip().

Changes in v2:
- Added new patch to fix integer truncation in do_read() and skip().
- Organized as patch series to separate the two security fixes.

v1: https://lore.kernel.org/linux-perf-users/20260722113552.191143-3-tshah@linux.ibm.com/

-- 
2.47.3

[PATCH v4 1/5] perf trace-event: Fix buffer overflow in read_string()

[Tanushree Shah]

read_string() writes into buf[BUFSIZ] one byte at a time without
checking 'size' against the buffer bound before each write. A
string longer than BUFSIZ in the input overflows the stack buffer.

Add a bounds check before each write to prevent overflow. On
overflow the function returns NULL, matching its other error paths.

Fixes: 9215545e99d8 ("perf: Convert perf tracing data into a tracing_data event")
Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/trace-event-read.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index ecbbb93f0185..afd458cf1387 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -127,6 +127,11 @@ static char *read_string(void)
 			}
 		}
 
+		if (size >= (int)sizeof(buf) - 1) {
+			pr_debug("string too long (max %zu bytes)", sizeof(buf) - 1);
+			goto out;
+		}
+
 		buf[size++] = c;
 
 		if (!c)
-- 
2.47.3

[PATCH v4 2/5] perf trace-event: Fix integer truncation in do_read() and skip()

[Tanushree Shah]

The do_read() and skip() functions use 'int' for size parameters,
truncating 64-bit sizes from callers. This causes two issues:

1. Uninitialized memory dump: do_read() reads fewer bytes than
   allocated, leaving uninitialized heap memory that gets written
   to output files.

2. Out-of-bounds read: Parsing functions process the full 64-bit
   size while only partial data was read into the buffer.

Change do_read(), __do_read(), and skip() to use size_t for size
parameters and ssize_t for return values (where applicable), matching
read()/write() system calls.
Update callers to use ssize_t for storing return values.

Fixes: 454c407ec17a ("perf tools: Get rid of read_or_die() in trace-event-read.c")
Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/trace-event-read.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index afd458cf1387..52ed496d92c3 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -25,18 +25,18 @@ static int input_fd;
 static ssize_t trace_data_size;
 static bool repipe;
 
-static int __do_read(int fd, void *buf, int size)
+static ssize_t __do_read(int fd, void *buf, size_t size)
 {
-	int rsize = size;
+	size_t rsize = size;
 
 	while (size) {
-		int ret = read(fd, buf, size);
+		ssize_t ret = read(fd, buf, size);
 
 		if (ret <= 0)
 			return -1;
 
 		if (repipe) {
-			int retw = write(STDOUT_FILENO, buf, ret);
+			ssize_t retw = write(STDOUT_FILENO, buf, ret);
 
 			if (retw <= 0 || retw != ret) {
 				pr_debug("repiping input file");
@@ -51,13 +51,13 @@ static int __do_read(int fd, void *buf, int size)
 	return rsize;
 }
 
-static int do_read(void *data, int size)
+static ssize_t do_read(void *data, size_t size)
 {
-	int r;
+	ssize_t r;
 
 	r = __do_read(input_fd, data, size);
 	if (r <= 0) {
-		pr_debug("reading input file (size expected=%d received=%d)",
+		pr_debug("reading input file (size expected=%zu received=%zd)",
 			 size, r);
 		return -1;
 	}
@@ -68,10 +68,10 @@ static int do_read(void *data, int size)
 }
 
 /* If it fails, the next read will report it */
-static void skip(int size)
+static void skip(size_t size)
 {
 	char buf[BUFSIZ];
-	int r;
+	size_t r;
 
 	while (size) {
 		r = size > BUFSIZ ? BUFSIZ : size;
@@ -202,7 +202,7 @@ static int read_header_files(struct tep_handle *pevent)
 	unsigned long long size;
 	char *header_page;
 	char buf[BUFSIZ];
-	int ret = 0;
+	ssize_t ret = 0;
 
 	if (do_read(buf, 12) < 0)
 		return -1;
@@ -250,7 +250,7 @@ static int read_header_files(struct tep_handle *pevent)
 
 static int read_ftrace_file(struct tep_handle *pevent, unsigned long long size)
 {
-	int ret;
+	ssize_t ret;
 	char *buf;
 
 	buf = malloc(size);
@@ -276,7 +276,7 @@ static int read_ftrace_file(struct tep_handle *pevent, unsigned long long size)
 static int read_event_file(struct tep_handle *pevent, char *sys,
 			   unsigned long long size)
 {
-	int ret;
+	ssize_t ret;
 	char *buf;
 
 	buf = malloc(size);
@@ -322,7 +322,7 @@ static int read_event_files(struct tep_handle *pevent)
 	int systems;
 	int count;
 	int i,x;
-	int ret;
+	ssize_t ret;
 
 	systems = read4(pevent);
 
@@ -350,7 +350,7 @@ static int read_saved_cmdline(struct tep_handle *pevent)
 {
 	unsigned long long size;
 	char *buf;
-	int ret;
+	ssize_t ret;
 
 	/* it can have 0 size */
 	size = read8(pevent);
-- 
2.47.3

[PATCH v4 3/5] perf trace-event: Avoid double free and leak in trace_event__cleanup()/trace_event__init()

[Tanushree Shah]

trace_event__cleanup() frees t->pevent but never clears the
pointer. It can be called twice on the same trace_event: once
from trace_report()'s error path, and again from
perf_session__delete() during session teardown, resulting in a
double free / use-after-free.

Separately, trace_event__init() overwrites t->pevent/t->plugin_list
without releasing any existing handle, leaking memory if it's
called more than once on the same struct. eg. via a perf.data
file with multiple PERF_RECORD_HEADER_TRACING_DATA headers.

Guard against re-entry by returning early if t->pevent is already
NULL, and clear it after cleanup so a repeat call is a safe no-op.
Call trace_event__cleanup() at the start of trace_event__init(),
so a repeated init releases any existing handle before allocating
a new one.

Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/trace-event.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index 6a8c66c64b70..000c1e1d68c1 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -26,7 +26,11 @@ static bool tevent_initialized;
 
 int trace_event__init(struct trace_event *t)
 {
-	struct tep_handle *pevent = tep_alloc();
+	struct tep_handle *pevent;
+
+	trace_event__cleanup(t);
+
+	pevent = tep_alloc();
 
 	if (pevent) {
 		t->plugin_list = tep_load_plugins(pevent);
@@ -63,8 +67,13 @@ int trace_event__register_resolver(struct machine *machine,
 
 void trace_event__cleanup(struct trace_event *t)
 {
+	if (!t->pevent)
+		return;
+
 	tep_unload_plugins(t->plugin_list, t->pevent);
 	tep_free(t->pevent);
+	t->pevent = NULL;
+	t->plugin_list = NULL;
 }
 
 /*
-- 
2.47.3

[PATCH v4 4/5] perf trace-event: Fix heap overflows in read_ftrace_printk()/read_saved_cmdline()

[Tanushree Shah]

Both functions read an attacker-controlled size directly from the
input file and pass size + 1 to malloc() before reading size bytes
into the result:

read_ftrace_printk(): size is an unsigned int from read4(). When
size == UINT_MAX, size + 1 overflows to 0, so malloc(0) returns a
minimal allocation while size itself remains UINT_MAX.

read_saved_cmdline(): size is an unsigned long long from read8().
When size == ULLONG_MAX, size + 1 overflows to 0 the same way.

In both cases, do_read(buf, size) then attempts to read the full,
unwrapped size into the tiny allocated buffer, a heap buffer
overflow.

This was previously masked by do_read()'s size parameter being
'int': passing these values truncated them, which the read()
syscall's own boundary checks rejected before any data was read.
Fixing that truncation (widening do_read() to size_t) is correct
on its own, but it removes this accidental protection and exposes
the pre-existing missing bounds check in both functions.

Reject the one value that causes the overflow before it's used, in
each function.

Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/trace-event-read.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 52ed496d92c3..53f1920c3fcb 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -15,6 +15,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
+#include <limits.h>
 
 #include "trace-event.h"
 #include "debug.h"
@@ -180,6 +181,11 @@ static int read_ftrace_printk(struct tep_handle *pevent)
 	if (!size)
 		return 0;
 
+	if (size == UINT_MAX) {
+		pr_debug("invalid ftrace printk size\n");
+		return -1;
+	}
+
 	buf = malloc(size + 1);
 	if (buf == NULL)
 		return -1;
@@ -357,6 +363,11 @@ static int read_saved_cmdline(struct tep_handle *pevent)
 	if (!size)
 		return 0;
 
+	if (size == ULLONG_MAX) {
+		pr_debug("invalid saved cmdline size");
+		return -1;
+	}
+
 	buf = malloc(size + 1);
 	if (buf == NULL) {
 		pr_debug("memory allocation failure\n");
-- 
2.47.3

[PATCH v4 5/5] perf trace-event: Fix infinite loop in skip()

[Tanushree Shah]

skip() ignores do_read()'s return value and unconditionally
subtracts the requested chunk size from 'size' on every iteration.
This was previously bounded by size being 'int': a maliciously
large 64-bit value was truncated on assignment, capping the loop
early by accident.

Now that size is size_t, a crafted file supplying a very large
size causes skip() to keep requesting BUFSIZ-sized reads and
subtracting BUFSIZ from size regardless of whether do_read()
actually succeeds, spinning indefinitely even after EOF or a read
error.

Check do_read()'s return value and break out of the loop on
failure or EOF, so forward progress is only counted when a read
actually succeeds.

Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/trace-event-read.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 53f1920c3fcb..db1622fc99c2 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -72,12 +72,16 @@ static ssize_t do_read(void *data, size_t size)
 static void skip(size_t size)
 {
 	char buf[BUFSIZ];
-	size_t r;
+	ssize_t ret;
 
 	while (size) {
-		r = size > BUFSIZ ? BUFSIZ : size;
-		do_read(buf, r);
-		size -= r;
+		size_t len = size > BUFSIZ ? BUFSIZ : size;
+
+		ret = do_read(buf, len);
+		if (ret <= 0)
+			break;
+
+		size -= ret;
 	}
 }
 
-- 
2.47.3