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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer truncation when passing 64-bit sizes to do_read() and skip() leads to parser stream corruption and potential out-of-bounds memory reads.
- [High] read_proc_kallsyms() attempts to skip the kallsyms payload using lseek() but ignores the return value, breaking parsing when perf.data is read from an unseekable stream like a pipe.
--

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

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

This commit adds a bounds check to read_string() to prevent a stack buffer
overflow when reading strings longer than BUFSIZ from the input. It ensures
the function returns NULL on overflow, matching other error 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
[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential integer truncation
when passing 64-bit sizes to do_read() and skip().

In tools/perf/util/trace-event-read.c, do_read() and skip() take an int:

static int do_read(void *data, int size)
{ ... }

static void skip(int size)
{ ... }

But in read_header_files(), a 64-bit size is passed directly to them:

static int read_header_files(struct tep_handle *pevent)
{
	unsigned long long size;
	...
	size = read8(pevent);
	...
	if (do_read(header_page, size) < 0) {
	...
	size = read8(pevent);
	skip(size);
}

Could this lead to silent truncation and parser stream corruption if a
malicious perf.data file provides a large 64-bit size (like 0x100000005)?

On 32-bit systems, malloc() would allocate a small buffer, while the parser
uses the full 64-bit size, causing an immediate out-of-bounds read. On 64-bit
systems, skip() truncates the size, skipping only a few bytes, which might
desync the parser and allow arbitrary data into subsequent parsing stages.


[Severity: High]
This is also a pre-existing issue, but does read_proc_kallsyms() properly
handle unseekable streams like pipes?

In tools/perf/util/trace-event-read.c:read_proc_kallsyms():

	lseek(input_fd, size, SEEK_CUR);

This lseek() is called without checking its return value or providing a
read-based fallback.

If a user pipes a perf.data file into the tool (e.g., perf report -i -),
the stream pointer is not advanced. Could subsequent parser functions
(like read_ftrace_printk()) interpret ASCII text from the kallsyms payload
as a 4-byte size header, resulting in massive arbitrary memory allocations
and tool crashes?

This appears to violate the subsystem rule which states that pipe streams
do not support seek and must be properly handled.

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