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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Integer overflow in read_saved_cmdline() leads to massive heap buffer overflow
--

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

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

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.

> 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

[ ... ]

> @@ -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);

[Severity: Critical]
This is a pre-existing issue, but can this lead to a massive heap buffer
overflow?

In read_saved_cmdline(), size is an 8-byte value read directly from the
untrusted file:

	size = read8(pevent);
	if (!size)
		return 0;

	buf = malloc(size + 1);
	if (buf == NULL)
		return -1;

	ret = do_read(buf, size);

If the size read from the file is ULLONG_MAX (or UINT_MAX on 32-bit systems),
the size + 1 expression will wrap to 0.

The malloc(0) call will then succeed and return a small heap chunk, but the
original unwrapped massive size is passed to do_read(), which will loop and
read an enormous amount of data directly into the tiny buffer.

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