Re: [PATCH v5 09/10] perf ui hists: Fix dso_filter reference leak and exit zoom cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Unconditional free of non-heap memory causes a crash when exiting the browser or replacing a command-line symbol filter.
- [Medium] Unconditionally clearing hists filters and un-eliding columns on browser exit permanently destroys persistent command-line configurations.
--

--- Patch [9]: [PATCH v5 09/10] perf ui hists: Fix dso_filter reference leak and exit zoom cleanup ---
commit 1af645505e5c140a87449557e914e317c6cf9b3b
Author: Ian Rogers <irogers@google.com>

perf ui hists: Fix dso_filter reference leak and exit zoom cleanup

In hists_browser__zoom_map(), the active dso is assigned as a raw pointer
to hists->dso_filter without acquiring a reference using dso__get(). This
patch properly acquires a reference when assigning the filter, and releases
it when clearing it, while also resetting the persistent hists structure
upon exit.

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
> index 89c3d3dc69a27..e0ef756d95367 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c

[ ... ]

> @@ -3491,6 +3492,16 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  out_free_stack:
>  	pstack__delete(browser->pstack);
>  	free_popup_actions(actions, MAX_OPTIONS);
> +	thread__zput(hists->thread_filter);
> +	dso__put((struct dso *)hists->dso_filter);
> +	hists->dso_filter = NULL;
> +	hists->socket_filter = -1;
> +	perf_hpp__set_elide(HISTC_DSO, false);
> +	perf_hpp__set_elide(HISTC_THREAD, false);
> +	perf_hpp__set_elide(HISTC_SOCKET, false);
> +	hists__filter_by_dso(hists);
> +	hists__filter_by_thread(hists);
> +	hists__filter_by_socket(hists);

[Severity: Medium]
Could unconditionally clearing the hists filters and un-eliding columns here
permanently destroy persistent command-line configurations?

If a user launches perf report with persistent global filters like
--socket-filter 0, hists->socket_filter is initialized. When navigating into
the TUI for a specific event and then exiting back to the main menu (tab
switching), this teardown path unconditionally clears the filters (e.g.,
hists->socket_filter = -1) and un-elides the columns.

Could this destroy the user's requested command-line configuration for the
remainder of the session?

>  	zfree(&hists->symbol_filter_str);

[Severity: Medium]
Will this unconditional free cause a crash when exiting the browser or
replacing a command-line symbol filter?

When a user runs perf report --symbol-filter <sym>, the argv pointer is
assigned directly without strdup():

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

When exiting the browser, or when applying a new inline filter inside the TUI
via evsel__hists_browse(), this zfree() attempts to free the non-heap argv
string. Can this trigger a glibc abort? 

This unconditional free was introduced by commit "perf ui hists: Fix stack
use-after-return in symbol_filter_str".

>  	hists__filter_by_symbol(hists);
>  out:
>  	hist_browser__delete(browser);

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