Re: [PATCH 02/13] perf header: Bump up the max number of command line args allowed

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Heap buffer overflow in process_cmdline due to unvalidated section size.
- [High] Out-of-bounds heap read and write due to missing null-termination in do_read_string.
--

commit 9eedbd20bb926c7a91d560c07ac34934b0d810bb
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Bump up the max number of command line args allowed

This commit increases the arbitrary maximum limit of command line arguments
allowed when processing perf headers to accommodate wildcard expansions.

Link: https://sashiko.dev/#/patchset/20260408172846.96360-1-acme%40kernel.org

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2795,8 +2795,11 @@ process_event_desc(struct feat_fd *ff, void *data __maybe_unused)

[ ... ]

> -// Some reasonable arbitrary max for the number of command line arguments
> -#define MAX_CMDLINE_NR 32768
> +/*
> + * Some arbitrary max for the number of command line arguments,
> + * Wildcards can expand and end up with tons of command line args.
> + */
> +#define MAX_CMDLINE_NR 1048576
>  
>  static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
>  {
>  	struct perf_env *env = &ff->ph->env;
>  	char *str, *cmdline = NULL, **argv = NULL;
>  	u32 nr, i, len = 0;
>  
>  	if (do_read_u32(ff, &nr))
>  		return -1;
>  
>  	if (nr > MAX_CMDLINE_NR)
>  		return -1;
>  
>  	env->nr_cmdline = nr;
>  
>  	cmdline = zalloc(ff->size + nr + 1);

This isn't a bug introduced by this commit, but could this allocation allow a
heap buffer overflow when parsing an untrusted perf.data file?

The buffer size relies on ff->size, which originates from the attacker
controlled section->size in the file header. If do_read_string() reads from
the file descriptor without enforcing that the bytes read stay within
ff->size, could an attacker set a tiny section->size to force a small
allocation, but provide large strings in the payload?

If so, the subsequent memcpy(argv[i], str, strlen(str) + 1) in
process_cmdline() could write unbounded data past the end of the cmdline
buffer.

Additionally, does do_read_string() guarantee null termination for the
strings it reads?

If the payload lacks a null byte and do_read_string() leaves the buffer
unterminated, calling strlen(str) on the result could scan out of bounds
into adjacent heap memory until it finds a null byte. The subsequent memcpy()
would then copy this out of bounds data into the cmdline buffer, resulting
in an out of bounds write.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=2