Re: [PATCH V2] tools/perf: Fix the check for parameterized field in event term

[Athira Rajeev <atrajeev@linux.ibm.com>]

> On 26 Apr 2026, at 10:22 AM, sashiko-bot@kernel.org wrote:
> 
> Sashiko AI review found 2 potential issue(s):
> - [High] Stack-based buffer overflow in `perf_pmu__for_each_event` due to unchecked `snprintf` truncation underflow.
> - [Medium] Meaningful code and commit message mismatch regarding the substring check.
> --
> 
> commit d8e4b8dc1bd869d8438cc91880ce9791dc581a82
> Author: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> tools/perf: Fix the check for parameterized field in event term
> 
> This commit fixes an issue where the check for parameterized fields in
> event terms was incorrect, causing them not to show in "perf list". It
> also updates buffer sizing logic to use scnprintf instead of snprintf to
> prevent out-of-bounds writes.
> 
>> Fix this check to use:
>> 
>> if (str_ends_with(alias->terms, "=?")) {
> 
> This isn't a bug, but the commit message states it is using
> str_ends_with(), while the implemented code actually uses
> !strstr(alias->terms, "=?").
> 
> The code implementation appears correct since parameterized terms might not
> be at the very end of the string, but the commit message does not match
> the code.

Thanks for catching that. I will update the commit message

> 
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 23337d2fa281..fad7ef4c8e5d 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
> [ ... ]
>> @@ -2129,15 +2129,19 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
>> pr_err("Failure to parse '%s' terms '%s': %d\n",
>> alias->name, alias->terms, ret);
>> parse_events_terms__exit(&terms);
>> - snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
>> + scnprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
>> return buf;
>> }
>> - used = snprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
>> + used = scnprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
>> 
>> list_for_each_entry(term, &terms.terms, list) {
>> + const char *name = term->config;
>> +
>> + if (!name)
>> + name = parse_events__term_type_str(term->type_term);
>> if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
>> - used += snprintf(buf + used, sub_non_neg(len, used),
>> - ",%s=%s", term->config,
>> + used += scnprintf(buf + used, sub_non_neg(len, used),
>> + ",%s=%s", name,
>> term->val.str);
>> }
> 
> The switch to scnprintf() here prevents buffer overflow within
> format_alias(), but does the immediate caller perf_pmu__for_each_event()
> also need this fix?
> 
> Looking at tools/perf/util/pmu.c:perf_pmu__for_each_event():
> 
>        char buf[1024];
>        ...
>        info.name = format_alias(buf, sizeof(buf), pmu, event, ...);
>        buf_used = strlen(buf) + 1;
>        ...
>        if (strlen(event->unit) || event->scale != 1.0) {
>                info.scale_unit = buf + buf_used;
>                buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
>                                "%G%s", event->scale, event->unit) + 1;
>        }
> 
> If snprintf truncates the string, it returns the number of characters it
> would have written. This causes buf_used to exceed sizeof(buf).
> 
> The subsequent size calculation sizeof(buf) - buf_used would then underflow
> to a massive unsigned value, leading to a stack buffer overflow on the
> next write.
> 
> Should the snprintf calls in perf_pmu__for_each_event() also be converted
> to scnprintf to prevent stack corruption?
I will address this in V3 by updating with scnprintf in perf_pmu__for_each_event() as well.

Thanks
Athira
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260426043006.48113-1-atrajeev@linux.ibm.com?part=1