Re: [PATCH v2 2/4] 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] Unbounded addition of popup menu options leads to a stack buffer overflow.
--

--- Patch [2]: [PATCH v2 2/4] perf ui hists: Remove duplicated thread in popup_action ---
commit 52ae50056f9a85a49a481078d9bef28046659aff
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
[ ... ]
> @@ -3202,7 +3201,7 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
>  					   verbose);
>  			continue;
>  		case 't':
> -			actions->thread = thread;
> +			actions->ms.thread = thread;

[Severity: High]
This isn't a bug introduced by this patch, but is there a potential stack
buffer overflow in evsel__hists_browse() when building the popup menu?

It looks like the options and actions arrays are defined with a fixed size:

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

However, when interacting with the UI on complex profiling data, various
add_*_opt functions append elements and advance nr_options without bounds
checking:

    skip_annotation:
        nr_options += add_annotate_type_opt(&actions[nr_options], ...);
        nr_options += add_thread_opt(browser, &actions[nr_options], ...);
        ...
        if (hists__has(hists, sym) && browser->selection->sym) {
            nr_options += add_script_opt(browser, &actions[nr_options], ...);

Could this write out of bounds if the total number of added options exceeds 16?

>  			do_zoom_thread(browser, actions);
>  			continue;

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