[PATCH v2 0/2] perf trace-event: Fix security vulnerabilities in trace-event-read.c

[PATCH v2 0/2] perf trace-event: Fix security vulnerabilities in trace-event-read.c

[Tanushree Shah]

This series fixes two security issues in trace-event-read.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 functions using
   'int' parameters, causing uninitialized memory to be dumped and
   parsers to read out of bounds.

Both are pre-existing bugs; this series fixes them.

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/

Tanushree Shah (2):
  perf trace-event: Fix buffer overflow in read_string()
  perf trace-event: Fix integer truncation in do_read() and skip()

 tools/perf/util/trace-event-read.c | 33 +++++++++++++++++-------------
 1 file changed, 19 insertions(+), 14 deletions(-)

-- 
2.47.3

[PATCH v2 1/2] 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

Re: [PATCH v2 1/2] perf trace-event: Fix buffer overflow in read_string()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Error path in trace_report() leads to a double free of tevent->pevent when session initialization fails.
- [High] Integer truncation in do_read() allows a malicious trace file to cause the parser to process uninitialized heap memory.
--

--- Patch [1]: [PATCH v2 1/2] perf trace-event: Fix buffer overflow in read_string() ---
commit fca4f0cc3071265252215647838a7b30c9c0baaf
Author: Tanushree Shah <tshah@linux.ibm.com>

perf trace-event: Fix buffer overflow in read_string()

This patch fixes a potential stack buffer overflow in read_string() by
adding a bounds check before writing each byte into the buffer. If the
string exceeds the buffer size, it logs a debug message and returns NULL,
matching other error handling paths.

> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index ecbbb93f01853..afd458cf1387d 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;

[Severity: High]
This is a pre-existing issue, but could this error path trigger a double free
of tevent->pevent?

If read_string() returns NULL due to the bounds check, the error propagates
and trace_report() jumps to its error path:

tools/perf/util/trace-event-read.c:trace_report() {
    ...
out:
	if (pevent)
		trace_event__cleanup(tevent);
    ...
}

trace_event__cleanup() frees t->pevent but leaves the pointer dangling:

tools/perf/util/trace-event.c:trace_event__cleanup() {
    ...
	tep_free(t->pevent);
    ...
}

Later, during session teardown, perf_session__delete() unconditionally calls
trace_event__cleanup(&session->tevent), which will attempt to free the same
pointer again.

[Severity: High]
This is also a pre-existing issue, but does do_read() suffer from an integer
truncation issue?

Functions like read_event_file() allocate memory based on a 64-bit size:

tools/perf/util/trace-event-read.c:read_event_file() {
    ...
	ret = do_read(buf, size);
	if (ret < 0)
		goto out;

	ret = parse_event_file(pevent, buf, size, sys);
    ...
}

But do_read() takes size as an int:

tools/perf/util/trace-event-read.c:do_read() {
static int do_read(void *data, int size)
...
}

If a maliciously crafted perf.data provides a size larger than 32 bits, the
size is truncated when passed to do_read(), causing it to read fewer bytes.
Since read_event_file() only checks if ret < 0 rather than verifying
ret == size, it will pass the large, mostly uninitialized buffer to
parse_event_file().

> +		}
> +
>  		buf[size++] = c;
>  
>  		if (!c)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723093958.416650-3-tshah@linux.ibm.com?part=1

[PATCH v2 2/2] 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Heap buffer overflow in `read_ftrace_printk` unmasked by `size_t` conversion.
- [High] Infinite loop in `skip()` leading to Denial of Service.
--

--- Patch [2]: [PATCH v2 2/2] perf trace-event: Fix integer truncation in do_read() and skip() ---
commit b30cf605f4d2ba7957c47c5106a821726355dd7e
Author: Tanushree Shah <tshah@linux.ibm.com>

perf trace-event: Fix integer truncation in do_read() and skip()

This commit modifies do_read(), __do_read(), and skip() to use size_t and
ssize_t instead of int for sizes and return values. This addresses integer
truncation of 64-bit sizes, fixing uninitialized memory dumps and out-of-bounds
reads in the perf tools.

> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index afd458cf1387d..52ed496d92c3b 100644
> --- a/tools/perf/util/trace-event-read.c
> +++ b/tools/perf/util/trace-event-read.c
> @@ -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)

[Severity: Critical]
Does changing the size parameter to size_t unmask a heap buffer overflow in
read_ftrace_printk()?

If a crafted file supplies 0xFFFFFFFF, it triggers an overflow in
read_ftrace_printk():

read_ftrace_printk() {
        size = read4(pevent);
        ...
        buf = malloc(size + 1);
        if (buf == NULL)
                return -1;

        if (do_read(buf, size) < 0) {
        ...
}

Because size + 1 overflows to 0, malloc(0) returns a minimal heap chunk.

Previously, passing 0xFFFFFFFF to do_read() truncated the size parameter
to -1. When this -1 was passed to the read() syscall on 64-bit systems,
it became ULLONG_MAX. This failed boundary checks against TASK_SIZE_MAX,
causing the kernel to immediately return -EFAULT and preventing the read.

With size now being size_t, 0xFFFFFFFF is passed directly to the read()
syscall as 4294967295ULL. Because this is below the 64-bit TASK_SIZE_MAX
limit, the kernel will read up to MAX_RW_COUNT of file data into the tiny
allocated buffer.

Could this be prevented by adding a bounds check before the allocation?

>  {
> -	int r;
> +	ssize_t r;
>  
>  	r = __do_read(input_fd, data, 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;
>  		do_read(buf, r);
>  		size -= r;
>  	}
>  }

[Severity: High]
Does this change introduce an infinite loop when a maliciously crafted
perf.data file provides a massive 64-bit size?

Before the patch, passing ULLONG_MAX was implicitly truncated to -1.
In the first loop iteration, r = size > BUFSIZ ? BUFSIZ : size evaluated
-1 > 8192 as false, setting r = -1. Then size -= r set size to 0,
terminating the loop immediately.

Now that size is widened to size_t, skip() will continuously subtract
BUFSIZ from ULLONG_MAX. Because skip() ignores the return value of
do_read(), does this cause the perf process to spin endlessly even
after reaching EOF?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723093958.416650-3-tshah@linux.ibm.com?part=2