Re: [PATCH v5 10/10] perf annotate: Be robust to annotating without a thread

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Double-put of the `map` struct leading to a use-after-free when zooming out via the K_ESC/K_LEFT hotkeys.
--

--- Patch [10]: [PATCH v5 10/10] perf annotate: Be robust to annotating without a thread ---
commit 380963af7e8bad2a7eee6ec6e6e6656a899a7aa7
Author: Ian Rogers <irogers@google.com>

perf annotate: Be robust to annotating without a thread

This commit makes perf annotate more robust by trying harder to determine
the architecture for disassembly when a thread isn't given. It introduces
map_symbol__get_arch() to replace the now removed thread__get_arch() and
safely handles map_symbol fallbacks.

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index e0ef756d95367..aee3d2c4e1867 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
[ ... ]
> @@ -3341,14 +3341,16 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  			actions->ms.map = map;

[Severity: Medium]
Could this assignment to actions->ms.map lead to a use-after-free?

When zooming out via the K_ESC or K_LEFT hotkeys, actions->ms.map is assigned
the current map pointer without acquiring a reference via map__get().

The loop then continues to the top, where free_popup_actions() is called on
the actions array. 

Does free_popup_actions() systematically call map_symbol__exit(), causing it
to improperly drop a reference on actions[0].ms.map? This could lead to a
reference count underflow and use-after-free of the map.

>  			top = pstack__peek(browser->pstack);
>  			if (top == &browser->hists->dso_filter) {
>  				/*
>  				 * No need to set actions->dso here since
>  				 * it's just to remove the current filter.
>  				 */
>  				do_zoom_dso(browser, actions);
>  			} else if (top == &browser->hists->thread_filter) {
> -				actions->thread = thread;
> -				do_zoom_thread(browser, actions);
> +				memset(&hotkey_act, 0, sizeof(hotkey_act));
> +				hotkey_act.ms.thread = thread__get(thread);
> +				do_zoom_thread(browser, &hotkey_act);
> +				map_symbol__exit(&hotkey_act.ms);
>  			} else if (top == &browser->hists->socket_filter) {
>  				do_zoom_socket(browser, actions);
>  			}

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