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

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

[Athira Rajeev]

The format_alias() function in util/pmu.c has a check to
detect whether the event has parameterized field ( =? ).
The string alias->terms contains the event and if the event
has user configurable parameter, there will be presence of
sub string "=?" in the alias->terms.

Snippet of code:

 /* Paramemterized events have the parameters shown. */
       if (strstr(alias->terms, "=?")) {
               /* No parameters. */
               snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);

if "strstr" contains the substring, it returns a pointer
and hence enters the above check which is not the expected
check. And hence "perf list" doesn't have the parameterized
fields in the result.

Fix this check to use:

if (str_ends_with(alias->terms, "=?")) {

With this change, perf list shows the events correctly with
the strings showing parameters.

Other changes in this patch:
- Replace snprintf with scnprintf in buffer offset calculations to
ensure the 'used' count will not exceed the "len"
- If a parameterized event uses a built-in perf keyword for its
parameter name (eg, config=?), the lexer parses it as a predefined
term token, which sets term->config to NULL. Add check to use
parse_events__term_type_str() if term->config is NULL

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changelog:
v1 -> v2:
After review from Sashiko, added below changes:
Replace snprintf with scnprintf in buffer offset calculations to
ensure the 'used' count will not exceed the "len"
- If a parameterized event uses a built-in perf keyword for its
parameter name (eg, config=?), the lexer parses it as a predefined
term token, which sets term->config to NULL. Add check to use
parse_events__term_type_str() if term->config is NULL

 tools/perf/util/pmu.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

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
@@ -2117,7 +2117,7 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
 						   skip_duplicate_pmus);
 
 	/* Paramemterized events have the parameters shown. */
-	if (strstr(alias->terms, "=?")) {
+	if (!strstr(alias->terms, "=?")) {
 		/* No parameters. */
 		snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
 		return buf;
@@ -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);
 	}
 	parse_events_terms__exit(&terms);
-- 
2.47.3

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

[sashiko-bot]

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.

> 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?

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

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

[Athira Rajeev]

> 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