Re: [PATCH v4 4/9] perf ui hists: Fix uninitialized stack memory free on pstack allocation failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stack Use-After-Free (Dangling Pointer) in symbol filter string.
--

--- Patch [4]: [PATCH v4 4/9] perf ui hists: Fix uninitialized stack memory free on pstack allocation failure ---
commit afaaff3407afa197242fdd0b413eb9b246beadbe
Author: Ian Rogers <irogers@google.com>

perf ui hists: Fix uninitialized stack memory free on pstack allocation failure

Fixes heap corruption by initializing the options and actions arrays before
the pstack allocation check, preventing an uninitialized stack pointer from
being passed to free_popup_options() if the allocation fails.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-perf-users/20260709035230.6DBEE1F000E9@smtp.kernel.org/
Fixes: f2b487db45f2 ("perf hists browser: Fix possible memory leak")

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index c6cfbcc580617..8b4c7510991f3 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3064,15 +3064,15 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  		browser->min_pcnt = min_pcnt;
>  	hist_browser__update_nr_entries(browser);
>  

[Severity: High]
This is a pre-existing issue, but I noticed a potential stack use-after-free
involving the local variable buf that is defined in evsel__hists_browse().

Later in evsel__hists_browse(), the local stack array buf is assigned to
the persistent hists->symbol_filter_str:

hists->symbol_filter_str = *buf ? buf : NULL;

If the user presses TAB to switch to another event, evsel__hists_browse()
returns and pops its stack frame.

Can this dangling pointer be accessed by the background timer? It looks
like perf_top__sort_new_samples() periodically iterates over events and
calls hists__filter_entry_by_symbol(), which then dereferences the pointer:

tools/perf/util/hist.c:hists__filter_entry_by_symbol() {
    ...
    if (hists->symbol_filter_str != NULL &&
        (!he->ms.sym || strstr(he->ms.sym->name,
                   hists->symbol_filter_str) == NULL)) {
    ...
}

Could this read uninitialized or overwritten stack memory from the current
frame if a timer fires after the symbol filter string is set?

> +	memset(options, 0, sizeof(options));
> +	memset(actions, 0, sizeof(actions));
> +
>  	browser->pstack = pstack__new(3);
>  	if (browser->pstack == NULL)
>  		goto out;

[ ... ]

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