Re: [PATCH v6 1/2] tools: port perf ui from GTK 2 to GTK 4

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Stack buffer overflow in `perf_gtk__show_hierarchy` via unbounded `strcat` operations on a fixed-size 512-byte buffer.
- [Critical] Stack buffer overflow in `perf_gtk__add_hierarchy_entries` due to `advance_hpp` causing `hpp->size` to underflow.
--

commit 7496ddfd9ec629824a25a0325ee855c455c6681b
Author: Matt Turner <mattst88@gmail.com>

tools: port perf ui from GTK 2 to GTK 4

This commit ports the perf GTK UI to GTK 4. It replaces deprecated
widgets, updates signal handling to use GLib sources, and adjusts
dialogs to align with the new GTK 4 event loop paradigms.

> diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
> index bae21f336ae6d..716dcf02bd0e8 100644
> --- a/tools/perf/ui/gtk/hists.c
> +++ b/tools/perf/ui/gtk/hists.c

[Severity: Critical]
This is a pre-existing issue, but while reviewing the GTK 4 changes in
this file, I noticed a potential stack buffer overflow.

Does this code safely prevent overflowing the fixed-size 512-byte buffer?

The column formats are derived from the tracepoint fields in perf.data,
which could be crafted to contain an excessive number of columns or very
long headers. In perf_gtk__show_hierarchy():

        char buf[512];
        ...
        buf[0] = '\0';
        first_node = true;
        list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
                if (!first_node)
                        strcat(buf, " / ");
                ...
                perf_hpp_list__for_each_format(&fmt_node->hpp ,fmt) {
                        ...
                        if (!first_col)
                                strcat(buf, "+");
                        ...
                        fmt->header(fmt, &hpp, hists, 0, NULL);
                        strcat(buf, strim(hpp.buf));
                }
        }

Could this unbounded strcat() sequence overwrite the stack if the combined
headers exceed 512 bytes?


[Severity: Critical]
This is also a pre-existing issue, but I spotted a second potential stack
buffer overflow in this file.

Can the manual size accounting here cause hpp->size to underflow?

In perf_gtk__add_hierarchy_entries():

        perf_hpp_list__for_each_format(he->hpp_list, fmt) {
                int ret;

                if (fmt->color)
                        ret = fmt->color(fmt, hpp, he);
                else
                        ret = fmt->entry(fmt, hpp, he);

                snprintf(hpp->buf + ret, hpp->size - ret, "  ");
                advance_hpp(hpp, ret + 2);
        }

If the bytes written by fmt->entry() (stored in ret) are equal to or
slightly less than hpp->size, couldn't ret + 2 be strictly greater than
hpp->size?

If this occurs, advance_hpp() will subtract a value larger than hpp->size,
causing the unsigned hpp->size to underflow to SIZE_MAX.

Would subsequent loop iterations then use this unbounded size, allowing
formatting functions to overwrite the stack?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-perf-gtk2-v6-0-695d1c01aaa1@gmail.com?part=1