Re: [PATCH v1 2/3] perf ui hists: Remove duplicated thread in popup_action

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stack buffer overflow due to out-of-bounds writes on `actions` and `options` arrays.
--

--- Patch [2]: [PATCH v1 2/3] perf ui hists: Remove duplicated thread in popup_action ---
commit 8f959fa67798b84a03f0c7cd56908afc452f4cc7
Author: Ian Rogers <irogers@google.com>

perf ui hists: Remove duplicated thread in popup_action

struct popup_action has a thread but this is duplicated in the
map_symbol. Remove the non-map_symbol version so that there's only
ever 1 thread with a popup_action.

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 6353be9dba865..a93846b8dc885 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c

[ ... ]

> @@ -3305,7 +3304,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  				 */
>  				do_zoom_dso(browser, actions);
>  			} else if (top == &browser->hists->thread_filter) {
> -				actions->thread = thread;
> +				actions->ms.thread = thread;
>  				do_zoom_thread(browser, actions);
>  			} else if (top == &browser->hists->socket_filter) {
>  				do_zoom_socket(browser, actions);

[Severity: High]
This is a pre-existing issue, but can this code overflow the actions[] and options[] stack arrays later in evsel__hists_browse()?

The arrays are statically allocated on the stack with a maximum size of 16:

tools/perf/ui/browsers/hists.c:evsel__hists_browse() {
	#define MAX_OPTIONS  16
	char *options[MAX_OPTIONS];
	struct popup_action actions[MAX_OPTIONS];
	...
}

Further down in the function, nr_options is repeatedly incremented without bounds checking:

tools/perf/ui/browsers/hists.c:evsel__hists_browse() {
	...
	nr_options += add_annotate_opt(&actions[nr_options],
				       &options[nr_options],
				       &bi->from.ms,
				       bi->from.al_addr);
	...
}

If a user executes perf report with specific arguments on a crafted perf.data file, couldn't the number of options added exceed 16 and result in a stack buffer overflow?

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