Re: [PATCH 05/12] perf debuginfo: Show the debuginfod fetch progress and keys in the TUI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: terminal width underflow in ui__progress_window causes DoS
--

commit b92eb48b4032b30d5c7972471d8fd2b1f910094c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf debuginfo: Show the debuginfod fetch progress and keys in the TUI

This commit implements a TUI progress window for debuginfod fetches to
prevent the UI from appearing hung during long operations. It handles word
wrapping and keyboard interruptions in a constrained terminal space.

> diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
> index e4d322ce0b54c..c3274b92b0e22 100644
> --- a/tools/perf/ui/tui/util.c
> +++ b/tools/perf/ui/tui/util.c
[ ... ]
> @@ -272,3 +274,178 @@ struct perf_error_ops perf_tui_eops = {
>  	.error		= perf_tui__error,
>  	.warning	= perf_tui__warning,
>  };
[ ... ]
> +void ui__progress_window(const char *title, const char *text,
> +			 u64 fetched, u64 total)
> +{
[ ... ]
> +	ui__refresh_dimensions(false);
> +	mutex_lock(&ui__lock);
> +	inner = SLtt_Screen_Cols - 2;
> +	if (inner > 255)
> +		inner = 255;

[Severity: Medium]
What happens to inner if the terminal is very narrow and SLtt_Screen_Cols
is less than 2? Since inner is a signed integer and is computed without a
lower bound clamp, it can evaluate to a negative value.

> +	nr_lines = progress_window__wrap(text, inner, lines);
> +	height = nr_lines + 3;
[ ... ]
> +	SLsmg_gotorc(y++, 1);
> +	len = strlen(title);
> +	if (len > (size_t)inner)
> +		len = inner;

[Severity: Medium]
When inner is negative, it gets cast to a large size_t here. Doesn't this
bypass the truncation logic and allow the title to exceed the bounded width?

> +	SLsmg_write_nchars(title, len);
> +	for (i = 0; i < nr_lines; i++, y++) {
> +		SLsmg_gotorc(y, 1);
> +		SLsmg_write_nstring(lines[i], SLtt_Screen_Cols - 2);

[Severity: Medium]
If SLtt_Screen_Cols - 2 evaluates to a negative number, passing it directly
to an API expecting an unsigned length could trigger massive space padding.
Can this lead to a 100% CPU lockup or hang of the perf tool if a user resizes
the terminal to 1 column during a debuginfod fetch?

> +	}
> +	SLsmg_gotorc(y, 1);
> +	SLsmg_write_nstring(bytes, SLtt_Screen_Cols - 2);

[Severity: Medium]
Similarly, wouldn't passing the negative width here to the SLang library
cause the same issue for the bytes line?

> +	SLsmg_refresh();
> +	mutex_unlock(&ui__lock);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916183215.54305-1-acme@kernel.org?part=5