Re: [PATCH v4 1/9] perf hists browser: Increase MAX_OPTIONS to prevent stack buffer overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Uninitialized Pointer Free on error path in `evsel__hists_browse()`
--

--- Patch [1]: [PATCH v4 1/9] perf hists browser: Increase MAX_OPTIONS to prevent stack buffer overflow ---
commit e3c0cd11e9823ee7177a41443ce63fd50ac88289
Author: Ian Rogers <irogers@google.com>

perf hists browser: Increase MAX_OPTIONS to prevent stack buffer overflow

This commit increases MAX_OPTIONS from 16 to 32 in evsel__hists_browse() to
safely accommodate the maximum possible number of options and prevent a stack
buffer overflow.

Link: https://lore.kernel.org/linux-perf-users/20260708235834.3FB771F00A3A@smtp.kernel.org/

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index cfa6386e6e1da..671961946d56c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3003,7 +3003,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  	struct hists *hists = evsel__hists(evsel);
>  	struct hist_browser *browser = perf_evsel_browser__new(evsel, hbt, env);
>  	struct branch_info *bi = NULL;
> -#define MAX_OPTIONS  16
> +#define MAX_OPTIONS  32
>  	char *options[MAX_OPTIONS];

[Severity: Critical]
This isn't a bug introduced by this patch, but does the error path for
pstack__new() cause an arbitrary free here?

If pstack__new(3) fails due to ENOMEM in evsel__hists_browse(), execution
jumps to the out label before the options array is zeroed by memset():

tools/perf/ui/browsers/hists.c:evsel__hists_browse() {
    ...
    char *options[MAX_OPTIONS];
    ...
    browser->pstack = pstack__new(3);
    if (browser->pstack == NULL)
        goto out;
    ...
}

The error handler then calls free_popup_options(options, MAX_OPTIONS),
which attempts to free all 32 uninitialized stack pointers.

Could a crafted perf.data file potentially prime the stack and induce an
allocation failure to exploit this arbitrary free?

>  	struct popup_action actions[MAX_OPTIONS];
>  	int nr_options = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710024952.1668911-1-irogers@google.com?part=1