Re: [PATCH v6 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 1 potential issue(s) to consider:
- [High] Unconditional free of a non-heap `argv` pointer leading to a crash, and unintended destruction of the CLI symbol filter state.
--

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

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

In evsel__hists_browse(), the local stack array 'buf' is assigned directly
to the persistent 'hists->symbol_filter_str' pointer. This commit fixes the
dangling pointer by duplicating the string and cleanly freeing it on exit.

> 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]
If the user invoked perf report with a command line option like
--symbol-filter=my_func, hists->symbol_filter_str will have been assigned
directly to the argv pointer in tools/perf/builtin-report.c:

report__collapse_hists() {
    ...
    hists->symbol_filter_str = rep->symbol_filter_str;

Will calling zfree() here cause a glibc invalid pointer crash when the
user interactively applies a new filter?

> +				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: High]
Similarly, does this unconditional free on exit crash if the filter was
initialized from the CLI?

Additionally, does unconditionally clearing this pointer permanently destroy
the --symbol-filter state for subsequent views of the same event?

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

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