Re: [PATCH v5 01/10] 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:
- [High] A pointer to a local stack array is assigned to a longer-lived structure field (`hists->symbol_filter_str`), leading to a Use-After-Free.
--

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

perf hists browser: Increase MAX_OPTIONS to prevent stack buffer overflow

In evsel__hists_browse(), the options and actions arrays are statically
allocated on the stack with a size of MAX_OPTIONS. Depending on the context,
the sum of added options can theoretically exceed 16, potentially leading to
a stack buffer overflow. This patch increases MAX_OPTIONS to 32 to safely
accommodate the maximum possible number of options.

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];
>  	struct popup_action actions[MAX_OPTIONS];
>  	int nr_options = 0;

[Severity: High]
This isn't a bug introduced by this patch, but since we are looking at
evsel__hists_browse(), can this cause a use-after-free later?

Just below this context, there is a local stack array declared:

    char buf[128];

Later in the function, when handling the '/' key for symbol filtering,
a pointer to this local stack array is assigned to a persistent field:

    case '/':
        if (ui_browser__input_window("Symbol to show",
                ...
                buf, "ENTER: OK, ESC: Cancel",
                delay_secs * 2) == K_ENTER) {
            hists->symbol_filter_str = *buf ? buf : NULL;
            ...

When the function returns and the user exits the browser, buf is destroyed,
but hists->symbol_filter_str will still point to this invalid stack memory.

In a multi-threaded context like perf top, if background processing threads or
subsequent timer-driven UI refreshes (via hists__filter_by_symbol() ->
hists__filter_entry_by_symbol() -> strstr()) read from it:

    strstr(he->ms.sym->name, hists->symbol_filter_str)

Could this lead to memory corruption or crashes?

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