Re: [PATCH v7 3/3] perf tools gtk: fix two hierarchy-view stack buffer overflows

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Buffer state (`hpp->buf` and `hpp->size`) is permanently consumed across sibling nodes in the hierarchy, leading to buffer exhaustion, missing text output, and an out-of-bounds stack read (info leak).
--

commit 4426046073bc5d6534ae6439f098dee3dfc79459
Author: Matt Turner <mattst88@gmail.com>

perf tools gtk: fix two hierarchy-view stack buffer overflows

This commit fixes two stack buffer overflows in the perf GTK hierarchy view.
It replaces unbounded strcat calls with scnprintf in header building and
clamps the buffer advance in perf_gtk__add_hierarchy_entries.

> diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
> index 716dcf02bd0e8..342d4c3fecb01 100644
> --- a/tools/perf/ui/gtk/hists.c
> +++ b/tools/perf/ui/gtk/hists.c
> @@ -449,7 +449,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
>  		bf = hpp->buf;
>  		size = hpp->size;
>  		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
> -			int ret;
> +			int ret, inc;
>  
>  			if (fmt->color)
>  				ret = fmt->color(fmt, hpp, he);
> @@ -457,7 +457,18 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
>  				ret = fmt->entry(fmt, hpp, he);
>  
>  			snprintf(hpp->buf + ret, hpp->size - ret, "  ");
> -			advance_hpp(hpp, ret + 2);
> +			/*
> +			 * ret can be as large as hpp->size - 1, so ret + 2
> +			 * can exceed hpp->size. advance_hpp() doesn't clamp,
> +			 * so passing that through would underflow the
> +			 * size_t hpp->size and let a later fmt->entry() in
> +			 * this loop write past the end of the caller's
> +			 * stack buffer.
> +			 */
> +			inc = ret + 2;
> +			if (inc > (int)hpp->size)
> +				inc = hpp->size;
> +			advance_hpp(hpp, inc);
>  		}
>  
>  		gtk_tree_store_set(store, &iter, col_idx, strim(bf), -1);

[Severity: High]
This is a pre-existing issue, but does this logic permanently consume the buffer
for all subsequent sibling nodes in the hierarchy?

While this patch successfully fixes the underflow by clamping `inc`, the buffer
state (`hpp->buf` and `hpp->size`) is never restored at the end of the outer
loop for the next sibling iteration. 

Because the state is saved at the start of formatting (`bf = hpp->buf;`) but not
restored across the outer rb-tree traversal loop, the 512-byte buffer is
permanently consumed as the tree is traversed.

When the buffer is completely exhausted (`hpp->size` hits 0), `bf` points
past the end of the original 512-byte stack buffer. Doesn't this mean the
subsequent `strim(bf)` call will perform an out-of-bounds read on the stack,
leaking adjacent uninitialized memory into the UI?

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