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

[PATCH V4 1/2] 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 (!strstr(alias->terms, "=?")) {

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

Before the fix:

 # ./perf list|grep -w PM_PAU_CYC
  hv_24x7/PM_PAU_CYC/                                [Kernel PMU event]

With this fix:

 # ./perf list|grep -w PM_PAU_CYC
  hv_24x7/PM_PAU_CYC,chip=?/                         [Kernel PMU event]

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changelog:
v3 -> v4:
Updated commit message to show real example
addressing review comment from Namhyung.

v2 -> v3:
Split the strstr correction in a single patch

 tools/perf/util/pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 23337d2fa281..0b8d58543f17 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;
-- 
2.47.3

[PATCH V4 2/2] tools/perf: Use scnprintf in buffer offset calculations

[Athira Rajeev]

Replace snprintf with scnprintf in buffer offset calculations to
ensure the 'used' count will not exceed the "len".

The current logic in perf_pmu__for_each_event uses an unconditional
+ 1 increment to buf_used to account for null terminators. This can
cause a a stack buffer overflow in the subsequent scnprintf call.
When the local stack buffer buf (1024 bytes) is full, buf_used can
reach 1025. This causes the subsequent remaining space calculation
sizeof(buf) - buf_used to underflow.

Use sub_non_neg() to see if space actually existed, and only
increment the offset if remaning space is present.

Changes includes:
- Use sub_non_neg to check if space exists
- Replacing snprintf with scnprintf to ensure the return value
reflects the actual bytes written into the buffer.
- Only increment buf_used by 1 if space exists
- 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:
v2 -> v3:
- Split the scnprintf related changes in separate patch
- Handle the overflow issues and unconditional increment
wrapped around sub_non_neg addressing review comment from Sashiko

 tools/perf/util/pmu.c | 46 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0b8d58543f17..4b9ade1a4cf9 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);
 	}
 	parse_events_terms__exit(&terms);
@@ -2201,6 +2205,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct hashmap_entry *entry;
 	size_t bkt;
+	size_t size_rem, len;
 
 	if (perf_pmu__is_tracepoint(pmu))
 		return tp_pmu__for_each_event(pmu, state, cb);
@@ -2234,17 +2239,36 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 			}
 			buf_used = strlen(buf) + 1;
 		}
+
 		info.scale_unit = NULL;
 		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;
+			/* Check the remaining space */
+			size_rem = sub_non_neg(sizeof(buf), buf_used);
+
+			if (size_rem > 0) {
+				info.scale_unit = buf + buf_used;
+				len = scnprintf(buf + buf_used, size_rem, "%G%s",
+						event->scale, event->unit);
+				/*
+				 * Increment buf_used by 1 only if
+				 * it fits remaining space
+				 */
+				buf_used += min(len + 1, size_rem);
+			}
 		}
 		info.desc = event->desc;
 		info.long_desc = event->long_desc;
-		info.encoding_desc = buf + buf_used;
-		buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
-				"%.*s/%s/", (int)pmu_name_len, info.pmu_name, event->terms) + 1;
+		info.encoding_desc = NULL;
+
+		/* Check the remaining space */
+		size_rem = sub_non_neg(sizeof(buf), buf_used);
+		if (size_rem > 0) {
+			info.encoding_desc = buf + buf_used;
+			len = scnprintf(buf + buf_used, size_rem, "%.*s/%s/",
+					(int)pmu_name_len, info.pmu_name, event->terms);
+			buf_used += min(len + 1, size_rem);
+		}
+
 		info.str = event->terms;
 		info.topic = event->topic;
 		info.deprecated = perf_pmu_alias__check_deprecated(pmu, event);
@@ -2254,7 +2278,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	}
 	if (pmu->selectable) {
 		info.name = buf;
-		snprintf(buf, sizeof(buf), "%s//", pmu->name);
+		scnprintf(buf, sizeof(buf), "%s//", pmu->name);
 		info.alias = NULL;
 		info.scale_unit = NULL;
 		info.desc = NULL;
-- 
2.47.3

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

[Venkat]

> On 4 May 2026, at 9:12 PM, Athira Rajeev <atrajeev@linux.ibm.com> wrote:
> 
> 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 (!strstr(alias->terms, "=?")) {
> 
> With this change, perf list shows the events correctly with
> the strings showing parameters.
> 
> Before the fix:
> 
> # ./perf list|grep -w PM_PAU_CYC
>  hv_24x7/PM_PAU_CYC/                                [Kernel PMU event]
> 
> With this fix:
> 
> # ./perf list|grep -w PM_PAU_CYC
>  hv_24x7/PM_PAU_CYC,chip=?/                         [Kernel PMU event]
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> ---
> Changelog:
> v3 -> v4:
> Updated commit message to show real example
> addressing review comment from Namhyung.
> 
> v2 -> v3:
> Split the strstr correction in a single patch
> 
> tools/perf/util/pmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 23337d2fa281..0b8d58543f17 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;
> -- 
> 2.47.3
> 

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

[Ian Rogers]

On Tue, May 12, 2026 at 1:53 AM Venkat <venkat88@linux.ibm.com> wrote:
>
>
>
> > On 4 May 2026, at 9:12 PM, Athira Rajeev <atrajeev@linux.ibm.com> wrote:
> >
> > 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 (!strstr(alias->terms, "=?")) {
> >
> > With this change, perf list shows the events correctly with
> > the strings showing parameters.
> >
> > Before the fix:
> >
> > # ./perf list|grep -w PM_PAU_CYC
> >  hv_24x7/PM_PAU_CYC/                                [Kernel PMU event]
> >
> > With this fix:
> >
> > # ./perf list|grep -w PM_PAU_CYC
> >  hv_24x7/PM_PAU_CYC,chip=?/                         [Kernel PMU event]
> >
> > Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
>
> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> > ---
> > Changelog:
> > v3 -> v4:
> > Updated commit message to show real example
> > addressing review comment from Namhyung.
> >
> > v2 -> v3:
> > Split the strstr correction in a single patch
> >
> > tools/perf/util/pmu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 23337d2fa281..0b8d58543f17 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;
> > --
> > 2.47.3
> >
>

Re: [PATCH V4 2/2] tools/perf: Use scnprintf in buffer offset calculations

[Ian Rogers]

On Mon, May 4, 2026 at 8:42 AM Athira Rajeev <atrajeev@linux.ibm.com> wrote:
>
> Replace snprintf with scnprintf in buffer offset calculations to
> ensure the 'used' count will not exceed the "len".
>
> The current logic in perf_pmu__for_each_event uses an unconditional
> + 1 increment to buf_used to account for null terminators. This can
> cause a a stack buffer overflow in the subsequent scnprintf call.
> When the local stack buffer buf (1024 bytes) is full, buf_used can
> reach 1025. This causes the subsequent remaining space calculation
> sizeof(buf) - buf_used to underflow.
>
> Use sub_non_neg() to see if space actually existed, and only
> increment the offset if remaning space is present.
>
> Changes includes:
> - Use sub_non_neg to check if space exists
> - Replacing snprintf with scnprintf to ensure the return value
> reflects the actual bytes written into the buffer.
> - Only increment buf_used by 1 if space exists
> - 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
> Changelog:
> v2 -> v3:
> - Split the scnprintf related changes in separate patch
> - Handle the overflow issues and unconditional increment
> wrapped around sub_non_neg addressing review comment from Sashiko
>
>  tools/perf/util/pmu.c | 46 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 0b8d58543f17..4b9ade1a4cf9 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);
>         }
>         parse_events_terms__exit(&terms);
> @@ -2201,6 +2205,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
>         int ret = 0;
>         struct hashmap_entry *entry;
>         size_t bkt;
> +       size_t size_rem, len;
>
>         if (perf_pmu__is_tracepoint(pmu))
>                 return tp_pmu__for_each_event(pmu, state, cb);
> @@ -2234,17 +2239,36 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
>                         }
>                         buf_used = strlen(buf) + 1;
>                 }
> +
>                 info.scale_unit = NULL;
>                 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;
> +                       /* Check the remaining space */
> +                       size_rem = sub_non_neg(sizeof(buf), buf_used);
> +
> +                       if (size_rem > 0) {
> +                               info.scale_unit = buf + buf_used;
> +                               len = scnprintf(buf + buf_used, size_rem, "%G%s",
> +                                               event->scale, event->unit);
> +                               /*
> +                                * Increment buf_used by 1 only if
> +                                * it fits remaining space
> +                                */
> +                               buf_used += min(len + 1, size_rem);
> +                       }
>                 }
>                 info.desc = event->desc;
>                 info.long_desc = event->long_desc;
> -               info.encoding_desc = buf + buf_used;
> -               buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
> -                               "%.*s/%s/", (int)pmu_name_len, info.pmu_name, event->terms) + 1;
> +               info.encoding_desc = NULL;
> +
> +               /* Check the remaining space */
> +               size_rem = sub_non_neg(sizeof(buf), buf_used);
> +               if (size_rem > 0) {
> +                       info.encoding_desc = buf + buf_used;
> +                       len = scnprintf(buf + buf_used, size_rem, "%.*s/%s/",
> +                                       (int)pmu_name_len, info.pmu_name, event->terms);
> +                       buf_used += min(len + 1, size_rem);
> +               }
> +
>                 info.str = event->terms;
>                 info.topic = event->topic;
>                 info.deprecated = perf_pmu_alias__check_deprecated(pmu, event);
> @@ -2254,7 +2278,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
>         }
>         if (pmu->selectable) {
>                 info.name = buf;
> -               snprintf(buf, sizeof(buf), "%s//", pmu->name);
> +               scnprintf(buf, sizeof(buf), "%s//", pmu->name);
>                 info.alias = NULL;
>                 info.scale_unit = NULL;
>                 info.desc = NULL;
> --
> 2.47.3
>