Re: [PATCH v5 53/58] perf: Remove libperl support, legacy Perl scripts and tests

sashiko-bot
2026-04-24T17:47:36Z

Sashiko AI review found 2 potential issue(s):
- [Critical] A stack buffer overflow occurs in `check_ev_match()` when parsing an event name from a script's `-record` file.
- [Critical] Improper size calculations and missing bounds checks lead to an exploitable heap buffer overflow in `find_scripts()` when extracting script names.
--

commit 573b39340e3ff8ff394b5f6ed03e2d363a352ad4
Author: Ian Rogers <irogers@google.com>

perf: Remove libperl support, legacy Perl scripts and tests

This commit removes embedded Perl interpreter support from perf, as all
legacy Perl scripts have been ported to Python or are no longer needed.
The changes include removing libperl feature detection, build rules,
and the scripts themselves.

[ ... ]

> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 1e8c2c2f952d42..27a099af2135e3 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -200,14 +200,13 @@ static int find_scripts(char **scripts_array, char **scripts_path_array, int num
> if (!strcmp(lang_dirent->d_name, ".") || !strcmp(lang_dirent->d_name, ".."))
> continue;
>
> -#ifndef HAVE_LIBPERL_SUPPORT
> - if (strstr(lang_dirent->d_name, "perl"))
> - continue;
> -#endif
> +
> #ifndef HAVE_LIBPYTHON_SUPPORT
> if (strstr(lang_dirent->d_name, "python"))
> continue;
> #endif
> + if (strstr(lang_dirent->d_name, "perl"))
> + continue;
>
> lang_dir_fd = openat(scripts_dir_fd, lang_dirent->d_name, O_DIRECTORY);
> if (lang_dir_fd == -1)

These aren't regressions introduced by this patch, but while reviewing this
area I noticed two pre-existing issues nearby that could lead to buffer
overflows.

First, does the extraction of script names in find_scripts() have a bounds
checking issue?

The size passed to snprintf() is derived from the input filename's length
rather than the destination buffer's fixed size:

tools/perf/ui/browsers/scripts.c:find_scripts() {
...
while ((script_dirent = readdir(lang_dir)) != NULL) {
...
temp = strchr(script_dirent->d_name, '.');
snprintf(scripts_array[i],
(temp - script_dirent->d_name) + 1,
"%s", script_dirent->d_name);
...
}

If a script filename exceeds 127 characters, snprintf() will write past the
128-byte boundary of scripts_array[i] and overwrite the adjacent
scripts_path_array[i] buffer, leading to a heap buffer overflow.

Additionally, if the filename lacks a dot, temp evaluates to NULL, which
causes an integer underflow that disables snprintf()'s truncation entirely.

Second, does check_ev_match() overflow the event name buffer when parsing a
script's -record file?

The length of the event name is calculated and then directly passed to
snprintf() with a fixed 128-byte stack array:

tools/perf/ui/browsers/scripts.c:check_ev_match() {
...
while (strlen(p)) {
...
char evname[128];

p = strstr(p, "-e");
...
p += 2;
p = skip_spaces(p);
len = strcspn(p, " \t");
...
snprintf(evname, len + 1, "%s", p);
...
}

If the event name in the -record file exceeds 127 characters, snprintf() will
write beyond the bounds of the array, resulting in a stack buffer overflow.
An attacker could exploit this by placing a maliciously crafted -record file
in the script directory.

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