Re: [PATCH v4 6/9] perf ui hists: Fix dso_filter reference leak and exit cleanup

[sashiko-bot@kernel.org]

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

New issues:
- [High] Clearing filter pointers on browser exit without reverting the corresponding filter state permanently corrupts the UI and histogram entries.

Pre-existing issues:
- [High] The `hists->symbol_filter_str` pointer is dangerously assigned to a local stack array `buf`, resulting in a dangling pointer when the function returns.
--

--- Patch [6]: [PATCH v4 6/9] perf ui hists: Fix dso_filter reference leak and exit cleanup ---
commit 817d491dc7ee2892f5d2ddca3b8801a25555bb12
Author: Ian Rogers <irogers@google.com>
Subject: perf ui hists: Fix dso_filter reference leak and exit cleanup

This commit aims to fix a potential use-after-free defect and reference
leak in the perf hists browser. It ensures dso_filter references are
properly acquired and released, and safely clears filters upon exiting
the browser.

Closes: https://lore.kernel.org/linux-perf-users/20260709170834.52F1A1F000E9@smtp.kernel.org/

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
[ ... ]
> 				hists->symbol_filter_str = *buf ? buf : NULL;

[Severity: High]
This is a pre-existing issue, but might assigning the long-lived
hists->symbol_filter_str pointer directly to a local stack array (buf)
cause a use-after-return bug?

If the browser is re-entered, or in perf top where a background thread
continuously processes new samples and re-evaluates filters via
hists__filter_entry_by_symbol(), it will call strstr() on garbage
stack memory.

Could this lead to random filtering behavior, information leaks, or
crashes?

[ ... ]
> @@ -3450,6 +3451,9 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  	}
>  out_free_stack:
>  	pstack__delete(browser->pstack);
> +	thread__zput(hists->thread_filter);
> +	dso__put((struct dso *)hists->dso_filter);
> +	hists->dso_filter = NULL;

[Severity: High]
Will clearing these filter pointers without reverting the corresponding
filter state leave the interface in a corrupted state?

When a user zooms into a dso or thread and exits the browser, the active
filters are nullified here but the histogram entries remain filtered
(he->filtered) and columns remain elided (perf_hpp__set_elide).

When the user re-enters the browser, they are presented with a filtered
view but have no way to 'Zoom out' because the filter pointers are now
null, replacing the 'Zoom out' menu options with 'Zoom in'.

Could this permanently break the browser functionality and corrupt the
persistent state of the hists object across browser sessions?

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

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