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

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Heap buffer overflow in `process_cmdline` due to unbounded file reads into an undersized allocation.
--

commit 3f19c967de1b22045b10b4bbef5f04c6e3827352
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

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

This commit increases the upper limit validation for command line
arguments from 32,768 to 1,048,576 to accommodate large 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
> index a2796b72adc4d..22c44b6f0b098 100644
> --- 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)
>  	return 0;
>  }
>  
> -// 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

While this patch doesn't introduce the underlying issue, does bumping
this limit exacerbate a regression involving a heap buffer overflow in
process_cmdline?

Currently, process_cmdline allocates the buffer based on ff->size:

    cmdline = zalloc(ff->size + nr + 1);

When parsing directly from the file descriptor, do_read_string uses
__do_read_fd, which reads lengths and string data from the file without
capping the total bytes read to ff->size.

If a maliciously crafted perf.data file provides a small ff->size but
supplies arbitrarily large strings in the payload, could do_read_string
read them successfully, causing the subsequent memcpy loop to overflow
the cmdline allocation?

Additionally, since do_read_string lacks null-termination enforcement,
if a string is provided without a null byte, could strlen perform an
out-of-bounds heap over-read, further inflating the length of the string
being copied?

>  static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
>  {

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