Re: [PATCH v5 04/10] perf ui hists: Fix stack use-after-return in symbol_filter_str

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling `zfree()` on `hists->symbol_filter_str` attempts to free a non-heap pointer (`argv`), causing `perf report` to crash.
- [Medium] Exiting the browser view permanently and unintentionally deletes the global command-line symbol filter.
--

--- Patch [4]: [PATCH v5 04/10] perf ui hists: Fix stack use-after-return in symbol_filter_str ---
commit d9e40115ffaf92f2da5c8c579054c376ce729ed4
Author: Ian Rogers <irogers@google.com>

perf ui hists: Fix stack use-after-return in symbol_filter_str

The patch duplicates the input string using strdup() to prevent a stack
use-after-return, safely invoking zfree() when modifying the filter, and
resetting the filter string upon exiting the browser.

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 793959574a9ad..4282ef1b81f75 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3213,7 +3213,10 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  					"To remove the filter later, press / + ENTER.",
>  					buf, "ENTER: OK, ESC: Cancel",
>  					delay_secs * 2) == K_ENTER) {
> -				hists->symbol_filter_str = *buf ? buf : NULL;
> +				char *new_filter = *buf ? strdup(buf) : NULL;
> +
> +				zfree(&hists->symbol_filter_str);

[Severity: High]
Will calling zfree() here crash perf if the symbol filter was provided via
the command line?

When a user runs perf report with a command line filter, the argv pointer is
eventually assigned to the hists struct:

tools/perf/builtin-report.c:cmd_report() {
    ...
    report.symbol_filter_str = argv[0];
    ...
}

tools/perf/builtin-report.c:report__collapse_hists() {
    ...
    hists->symbol_filter_str = rep->symbol_filter_str;
    ...
}

Attempting to free this non-heap pointer in evsel__hists_browse() would
trigger an invalid free abort in glibc.

> +				hists->symbol_filter_str = new_filter;
>  				hists__filter_by_symbol(hists);
>  				hist_browser__reset(browser);
>  			}

[ ... ]

> @@ -3447,6 +3450,8 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  	}
>  out_free_stack:
>  	pstack__delete(browser->pstack);
> +	zfree(&hists->symbol_filter_str);
> +	hists__filter_by_symbol(hists);

[Severity: Medium]
Does this unconditional cleanup unintentionally destroy the global
command-line filter state upon exiting the UI function?

If a user opens perf report with a command-line filter, views an event's
histogram, and then exits back to the main menu, out_free_stack clears the
filter. If they subsequently re-enter the event's histogram, the view will
be unexpectedly unfiltered.

>  out:
>  	hist_browser__delete(browser);
>  	free_popup_options(options, MAX_OPTIONS);

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