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

[PATCH] 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.

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
 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] tools/perf: Fix the check for parameterized field in event term

[Venkat]

> On 14 Mar 2026, at 2:03 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.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> ---
> 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
> 

Tested this patch, and its working as expected.

Before Patch:

./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
  hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]

After Patch:

./perf list hv_24x7 | grep -i CPM_EXT_INT_OS 
 hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]


./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/


 Performance counter stats for 'system wide':

        2018866563      hv_24x7/PM_PAU_CYC,chip=0/                                            

     229.938231314 seconds time elapsed

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>

Regards,
Venkat.

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

[Ian Rogers]

On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88@linux.ibm.com> wrote:
>
> > On 14 Mar 2026, at 2:03 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.
> >
> > Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>

Thanks Athira, Sashiko is noting in its review:
https://sashiko.dev/#/patchset/20260314083304.75321-1-atrajeev%40linux.ibm.com

By inverting this check, parameterized events now proceed to
parse_events_terms() and the rest of format_alias().

If a parameterized event uses a built-in perf keyword for its parameter name
(e.g., config=?), the lexer parses it as a predefined term token, which sets
term->config to NULL.

Does this cause a NULL pointer dereference in the term iteration loop?

list_for_each_entry(term, &terms.terms, list) {
  if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
    used += snprintf(buf + used, sub_non_neg(len, used),
      ",%s=%s", term->config,
      term->val.str);
}

Passing a NULL pointer to the %s format specifier for term->config might
cause a segmentation fault or formatting corruption.

Can this also lead to a stack buffer overflow due to how snprintf() return
values are accumulated?

snprintf() returns the number of characters that would have been written.
If the event names and terms are long enough, the "used" variable can exceed
"len" (which is typically 1024 bytes).

When evaluating "buf + used" in the same loop:

  used += snprintf(buf + used, sub_non_neg(len, used), ...);

If "used" exceeds 1024, this creates an out-of-bounds pointer. Modern
compilers can use this undefined behavior to infer that "used" <= 1024
must be true, which optimizes out the safety check inside sub_non_neg():

static int sub_non_neg(int a, int b)
{
  if (b > a)
    return 0;
  return a - b;
}

The compiler can reduce this to simply "a - b" (or "len - used").
If "used" is actually greater than 1024, "len - used" evaluates to a
negative integer. This negative value is implicitly cast to size_t for
snprintf(), resulting in a massive size limit.

Would this cause snprintf() to write past the end of the stack buffer
without bounds checking? Using scnprintf() might prevent "used" from
exceeding "len".

Thanks,
Ian

> > ---
> > 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
> >
>
> Tested this patch, and its working as expected.
>
> Before Patch:
>
> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>   hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]
>
> After Patch:
>
> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>  hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]
>
>
> ./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/
>
>
>  Performance counter stats for 'system wide':
>
>         2018866563      hv_24x7/PM_PAU_CYC,chip=0/
>
>      229.938231314 seconds time elapsed
>
> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>
> Regards,
> Venkat.
>
>

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

[Athira Rajeev]

> On 17 Mar 2026, at 9:39 PM, Ian Rogers <irogers@google.com> wrote:
> 
> On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88@linux.ibm.com> wrote:
>> 
>>> On 14 Mar 2026, at 2:03 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.
>>> 
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> Thanks Athira, Sashiko is noting in its review:
> https://sashiko.dev/#/patchset/20260314083304.75321-1-atrajeev%40linux.ibm.com

Thanks Ian for pointing this. Its interesting to see this review.
I will check through the review.

Thanks
Athira
> 
> By inverting this check, parameterized events now proceed to
> parse_events_terms() and the rest of format_alias().
> 
> If a parameterized event uses a built-in perf keyword for its parameter name
> (e.g., config=?), the lexer parses it as a predefined term token, which sets
> term->config to NULL.
> 
> Does this cause a NULL pointer dereference in the term iteration loop?
> 
> list_for_each_entry(term, &terms.terms, list) {
>  if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
>    used += snprintf(buf + used, sub_non_neg(len, used),
>      ",%s=%s", term->config,
>      term->val.str);
> }
> 
> Passing a NULL pointer to the %s format specifier for term->config might
> cause a segmentation fault or formatting corruption.
> 
> Can this also lead to a stack buffer overflow due to how snprintf() return
> values are accumulated?
> 
> snprintf() returns the number of characters that would have been written.
> If the event names and terms are long enough, the "used" variable can exceed
> "len" (which is typically 1024 bytes).
> 
> When evaluating "buf + used" in the same loop:
> 
>  used += snprintf(buf + used, sub_non_neg(len, used), ...);
> 
> If "used" exceeds 1024, this creates an out-of-bounds pointer. Modern
> compilers can use this undefined behavior to infer that "used" <= 1024
> must be true, which optimizes out the safety check inside sub_non_neg():
> 
> static int sub_non_neg(int a, int b)
> {
>  if (b > a)
>    return 0;
>  return a - b;
> }
> 
> The compiler can reduce this to simply "a - b" (or "len - used").
> If "used" is actually greater than 1024, "len - used" evaluates to a
> negative integer. This negative value is implicitly cast to size_t for
> snprintf(), resulting in a massive size limit.
> 
> Would this cause snprintf() to write past the end of the stack buffer
> without bounds checking? Using scnprintf() might prevent "used" from
> exceeding "len".
> 
> Thanks,
> Ian
> 
>>> ---
>>> 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
>>> 
>> 
>> Tested this patch, and its working as expected.
>> 
>> Before Patch:
>> 
>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>>  hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]
>> 
>> After Patch:
>> 
>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>> hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]
>> 
>> 
>> ./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/
>> 
>> 
>> Performance counter stats for 'system wide':
>> 
>>        2018866563      hv_24x7/PM_PAU_CYC,chip=0/
>> 
>>     229.938231314 seconds time elapsed
>> 
>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>> 
>> Regards,
>> Venkat.

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

[Athira Rajeev]

> On 18 Mar 2026, at 12:05 PM, Athira Rajeev <atrajeev@linux.ibm.com> wrote:
> 
> 
> 
>> On 17 Mar 2026, at 9:39 PM, Ian Rogers <irogers@google.com> wrote:
>> 
>> On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88@linux.ibm.com> wrote:
>>> 
>>>> On 14 Mar 2026, at 2:03 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.
>>>> 
>>>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
>> 
>> Thanks Athira, Sashiko is noting in its review:
>> https://sashiko.dev/#/patchset/20260314083304.75321-1-atrajeev%40linux.ibm.com
> 
> Thanks Ian for pointing this. Its interesting to see this review.
> I will check through the review.
> 
> Thanks
> Athira
>> 
>> By inverting this check, parameterized events now proceed to
>> parse_events_terms() and the rest of format_alias().
>> 
>> If a parameterized event uses a built-in perf keyword for its parameter name
>> (e.g., config=?), the lexer parses it as a predefined term token, which sets
>> term->config to NULL.

Hi All,

sashiko brought up a scenario :

“If a parameterized event uses a built-in perf keyword for its parameter name
(e.g., config=?), the lexer parses it as a predefined term token, which sets
term->config to NULL."

Existing usage for parameterized events in perf:
======================================

In perf,  “?” is often used in event aliases to indicate that a parameter needs to be provided 
by the user (e.g., param1=?). This information is exposed via “format” in sysfs too.  The current way, 
how this is consumed in different PMU drivers is: example:

  # cat /sys/devices/pmu/format/param1
    config:12-15

   # cat /sys/devices/pmu/format/param2
    config1:0-17

Here param1 uses bits 12:15 in config and param2 used bits 0:17 in config1

And event will use these parameterized fields:
    #cat /sys/devices/pmu/events/event1       domain=1,param1=?,param2=?

“Perf list” expects "param1=?,param2=?” In the event listing
   # perf list|grep event1
   pmu/event1,param1=?,param2=?/                       [Kernel PMU event]

The patch here address fix in “strstr” check in format_alias for “perf list” to display these user expected “?” fields
in event alias.  Traditionally, perf treats config, config1, and config2 etc as hardcoded numeric
keywords or builtin keyword. 

The scneario sashiko brought up is interesting. Where a parameterized event
uses a built-in perf keyword  for its parameter name.

sashiko review : Scenario where parameterized event uses a built-in perf keyword 
============================================================
Example, having “config2” in sysfs format-driven logic. Below is an example case.

    #cat /sys/devices/pmu/events/event1
     config2=?,domain=1,param1=?,param2=?

      # cat /sys/devices/hv_24x7/format/config2         
         config2:0-16
  
 Here “perf list” will show as (null) or might crash too as sashiko review pointed out.

    # perf list|grep event1
    pmu/event1,(null)=?,param1=?,param2=?/                    [Kernel PMU event]

This is because term->config is null for built-in keywords. perf lexer sets this to NULL
for predefined keywords ( like 'config', 'period'), storing the type in the enum instead.

To fix displaying the “=?” in “perf list” for keywords like config2,  format_alias() function needs to handle
null term->config , ie by using parse_events__term_type_str() to map NULL config 
pointers back to their string keywords

But this only will fix the “perf list” part of it.  “Perf stat” or record will still fail since
currently these keywords are not considered as placeholders for “?” values.

If a PMU wants to use any builtin-keywords in parameters and want to ensure
it is set by user, this could be useful.  Again considering this as a use case, we need other changes
to allow these built-in keywords to function as placeholders (e.g., config2=?) within a PMU alias.

I am adding changes which I have identified needed for this support.
If it is better to post this separately as patch set and discuss, definitely will do that.
I wanted to run the idea of this new scenario with everyone. Apologies for the long text.

Thanks and looking for comments. 


From 21179a76be97ef81c69a9492f0f432dec5b5361c Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev@linux.ibm.com>
Date: Mon, 23 Mar 2026 08:38:30 -0400
Subject: [PATCH] tools/perf: Enable parameterized values for keywords like
 config1

PMU defines events that require user-supplied parameters (like chip
or core) are packed into specific bit-fields of the different config
registers. Traditionally, perf treats config, config1, and config2
as hardcoded numeric keywords, causing it to bypass the sysfs
format-driven bit-masking logic.

This patch allows these built-in keywords to function as placeholders
(e.g., config2=?) within a PMU alias.

Changes includes:
- Updated config_term_pmu in parse-events.c to recognize string
  placeholders for event terms. otherwise it will expect those to
  be of type PARSE_EVENTS__TERM_TYPE_NUM
- Updated pmu_config_term in pmu.c to make sure if keywords are used
  as placeholders , they pass through sysfs based format logic
 (pmu_find_format) and pmu_resolve_param_term() logic
- Modified format_alias() to ensure "perf list" display the parameterized
  events correctly
- Replace snprintf with scnprintf in buffer offset calculations to
   ensure the 'used' count will not exceed the "len"

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
 tools/perf/util/parse-events.c |  5 +++++
 tools/perf/util/pmu.c          | 37 +++++++++++++++++++++++++---------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b9efb296bba5..282b0a0a5340 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1051,6 +1051,11 @@ static int config_term_pmu(struct perf_event_attr *attr,
                 */
                return 0;
        }
+
+       if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR && term->val.str)
+               if (strstr(term->val.str, "?"))
+                       return 0;
+
        return config_term_common(attr, term, parse_state);
 }

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 23337d2fa281..2c4503e1a11b 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1442,10 +1442,19 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
                                  __u64 *value)
 {
        struct parse_events_term *t;
+       const char *name = term->config;
+
+       if (!term->config)
+               name = parse_events__term_type_str(term->type_term);

        list_for_each_entry(t, &head_terms->terms, list) {
+               const char *t_name = t->config;
+
+               if (!t_name)
+                       t_name = parse_events__term_type_str(t->type_term);
+
                if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM &&
-                   t->config && !strcmp(t->config, term->config)) {
+                   t_name && name && !strcmp(t_name, name)) {
                        t->used = true;
                        *value = t->val.num;
                        return 0;
@@ -1453,7 +1462,7 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
        }

        if (verbose > 0)
-               printf("Required parameter '%s' not specified\n", term->config);
+               printf("Required parameter '%s' not specified\n", name);

        return -1;
 }
@@ -1494,6 +1503,7 @@ static int pmu_config_term(const struct perf_pmu *pmu,
        struct perf_pmu_format *format;
        __u64 *vp;
        __u64 val, max_val;
+       const char *name;

        /*
         * If this is a parameter we've already used for parameterized-eval,
@@ -1507,7 +1517,8 @@ static int pmu_config_term(const struct perf_pmu *pmu,
         * traditionally have had to handle not having a PMU. An alias may
         * have hard coded config values, optionally apply them below.
         */
-       if (parse_events__is_hardcoded_term(term)) {
+       if (parse_events__is_hardcoded_term(term) &&
+                      term->type_val != PARSE_EVENTS__TERM_TYPE_STR) {
                /* Config terms set all bits in the config. */
                DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);

@@ -1576,7 +1587,11 @@ static int pmu_config_term(const struct perf_pmu *pmu,
                return 0;
        }

-       format = pmu_find_format(&pmu->format, term->config);
+       name = term->config;
+       if (!name)
+               name = parse_events__term_type_str(term->type_term);
+
+       format = pmu_find_format(&pmu->format, name);
        if (!format) {
                char *pmu_term = pmu_formats_string(&pmu->format);
                char *unknown_term;
@@ -2117,7 +2132,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 +2144,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



Thanks
Athira

>> 
>> Does this cause a NULL pointer dereference in the term iteration loop?
>> 
>> list_for_each_entry(term, &terms.terms, list) {
>> if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
>>   used += snprintf(buf + used, sub_non_neg(len, used),
>>     ",%s=%s", term->config,
>>     term->val.str);
>> }
>> 
>> Passing a NULL pointer to the %s format specifier for term->config might
>> cause a segmentation fault or formatting corruption.
>> 
>> Can this also lead to a stack buffer overflow due to how snprintf() return
>> values are accumulated?
>> 
>> snprintf() returns the number of characters that would have been written.
>> If the event names and terms are long enough, the "used" variable can exceed
>> "len" (which is typically 1024 bytes).
>> 
>> When evaluating "buf + used" in the same loop:
>> 
>> used += snprintf(buf + used, sub_non_neg(len, used), ...);
>> 
>> If "used" exceeds 1024, this creates an out-of-bounds pointer. Modern
>> compilers can use this undefined behavior to infer that "used" <= 1024
>> must be true, which optimizes out the safety check inside sub_non_neg():
>> 
>> static int sub_non_neg(int a, int b)
>> {
>> if (b > a)
>>   return 0;
>> return a - b;
>> }
>> 
>> The compiler can reduce this to simply "a - b" (or "len - used").
>> If "used" is actually greater than 1024, "len - used" evaluates to a
>> negative integer. This negative value is implicitly cast to size_t for
>> snprintf(), resulting in a massive size limit.
>> 
>> Would this cause snprintf() to write past the end of the stack buffer
>> without bounds checking? Using scnprintf() might prevent "used" from
>> exceeding "len".
>> 
>> Thanks,
>> Ian
>> 
>>>> ---
>>>> 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
>>>> 
>>> 
>>> Tested this patch, and its working as expected.
>>> 
>>> Before Patch:
>>> 
>>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>>> hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]
>>> 
>>> After Patch:
>>> 
>>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>>> hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]
>>> 
>>> 
>>> ./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/
>>> 
>>> 
>>> Performance counter stats for 'system wide':
>>> 
>>>       2018866563      hv_24x7/PM_PAU_CYC,chip=0/
>>> 
>>>    229.938231314 seconds time elapsed
>>> 
>>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>>> 
>>> Regards,
>>> Venkat.

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

[Ian Rogers]

On Mon, Mar 23, 2026 at 5:19 AM Athira Rajeev <atrajeev@linux.ibm.com> wrote:
>
>
>
> > On 18 Mar 2026, at 12:05 PM, Athira Rajeev <atrajeev@linux.ibm.com> wrote:
> >
> >
> >
> >> On 17 Mar 2026, at 9:39 PM, Ian Rogers <irogers@google.com> wrote:
> >>
> >> On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88@linux.ibm.com> wrote:
> >>>
> >>>> On 14 Mar 2026, at 2:03 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.
> >>>>
> >>>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> >>
> >> Thanks Athira, Sashiko is noting in its review:
> >> https://sashiko.dev/#/patchset/20260314083304.75321-1-atrajeev%40linux.ibm.com
> >
> > Thanks Ian for pointing this. Its interesting to see this review.
> > I will check through the review.
> >
> > Thanks
> > Athira
> >>
> >> By inverting this check, parameterized events now proceed to
> >> parse_events_terms() and the rest of format_alias().
> >>
> >> If a parameterized event uses a built-in perf keyword for its parameter name
> >> (e.g., config=?), the lexer parses it as a predefined term token, which sets
> >> term->config to NULL.
>
> Hi All,
>
> sashiko brought up a scenario :
>
> “If a parameterized event uses a built-in perf keyword for its parameter name
> (e.g., config=?), the lexer parses it as a predefined term token, which sets
> term->config to NULL."
>
> Existing usage for parameterized events in perf:
> ======================================
>
> In perf,  “?” is often used in event aliases to indicate that a parameter needs to be provided
> by the user (e.g., param1=?). This information is exposed via “format” in sysfs too.  The current way,
> how this is consumed in different PMU drivers is: example:
>
>   # cat /sys/devices/pmu/format/param1
>     config:12-15
>
>    # cat /sys/devices/pmu/format/param2
>     config1:0-17
>
> Here param1 uses bits 12:15 in config and param2 used bits 0:17 in config1
>
> And event will use these parameterized fields:
>     #cat /sys/devices/pmu/events/event1       domain=1,param1=?,param2=?
>
> “Perf list” expects "param1=?,param2=?” In the event listing
>    # perf list|grep event1
>    pmu/event1,param1=?,param2=?/                       [Kernel PMU event]
>
> The patch here address fix in “strstr” check in format_alias for “perf list” to display these user expected “?” fields
> in event alias.  Traditionally, perf treats config, config1, and config2 etc as hardcoded numeric
> keywords or builtin keyword.
>
> The scneario sashiko brought up is interesting. Where a parameterized event
> uses a built-in perf keyword  for its parameter name.
>
> sashiko review : Scenario where parameterized event uses a built-in perf keyword
> ============================================================
> Example, having “config2” in sysfs format-driven logic. Below is an example case.
>
>     #cat /sys/devices/pmu/events/event1
>      config2=?,domain=1,param1=?,param2=?
>
>       # cat /sys/devices/hv_24x7/format/config2
>          config2:0-16
>
>  Here “perf list” will show as (null) or might crash too as sashiko review pointed out.
>
>     # perf list|grep event1
>     pmu/event1,(null)=?,param1=?,param2=?/                    [Kernel PMU event]
>
> This is because term->config is null for built-in keywords. perf lexer sets this to NULL
> for predefined keywords ( like 'config', 'period'), storing the type in the enum instead.
>
> To fix displaying the “=?” in “perf list” for keywords like config2,  format_alias() function needs to handle
> null term->config , ie by using parse_events__term_type_str() to map NULL config
> pointers back to their string keywords
>
> But this only will fix the “perf list” part of it.  “Perf stat” or record will still fail since
> currently these keywords are not considered as placeholders for “?” values.
>
> If a PMU wants to use any builtin-keywords in parameters and want to ensure
> it is set by user, this could be useful.  Again considering this as a use case, we need other changes
> to allow these built-in keywords to function as placeholders (e.g., config2=?) within a PMU alias.
>
> I am adding changes which I have identified needed for this support.
> If it is better to post this separately as patch set and discuss, definitely will do that.
> I wanted to run the idea of this new scenario with everyone. Apologies for the long text.
>
> Thanks and looking for comments.
>
>
> From 21179a76be97ef81c69a9492f0f432dec5b5361c Mon Sep 17 00:00:00 2001
> From: Athira Rajeev <atrajeev@linux.ibm.com>
> Date: Mon, 23 Mar 2026 08:38:30 -0400
> Subject: [PATCH] tools/perf: Enable parameterized values for keywords like
>  config1
>
> PMU defines events that require user-supplied parameters (like chip
> or core) are packed into specific bit-fields of the different config
> registers. Traditionally, perf treats config, config1, and config2
> as hardcoded numeric keywords, causing it to bypass the sysfs
> format-driven bit-masking logic.
>
> This patch allows these built-in keywords to function as placeholders
> (e.g., config2=?) within a PMU alias.
>
> Changes includes:
> - Updated config_term_pmu in parse-events.c to recognize string
>   placeholders for event terms. otherwise it will expect those to
>   be of type PARSE_EVENTS__TERM_TYPE_NUM
> - Updated pmu_config_term in pmu.c to make sure if keywords are used
>   as placeholders , they pass through sysfs based format logic
>  (pmu_find_format) and pmu_resolve_param_term() logic
> - Modified format_alias() to ensure "perf list" display the parameterized
>   events correctly
> - Replace snprintf with scnprintf in buffer offset calculations to
>    ensure the 'used' count will not exceed the "len"
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> ---
>  tools/perf/util/parse-events.c |  5 +++++
>  tools/perf/util/pmu.c          | 37 +++++++++++++++++++++++++---------
>  2 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index b9efb296bba5..282b0a0a5340 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1051,6 +1051,11 @@ static int config_term_pmu(struct perf_event_attr *attr,
>                  */
>                 return 0;
>         }
> +
> +       if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR && term->val.str)
> +               if (strstr(term->val.str, "?"))
> +                       return 0;

nit: Since the nested if spans more than two lines, could you add
curly braces {} to the outer if statement. Could you also add a
comment explaining why the runtime parameter causes an early return
and why you used `strstr` instead of `strcmp`?

> +
>         return config_term_common(attr, term, parse_state);
>  }
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 23337d2fa281..2c4503e1a11b 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1442,10 +1442,19 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
>                                   __u64 *value)
>  {
>         struct parse_events_term *t;
> +       const char *name = term->config;
> +
> +       if (!term->config)

I think you meant "if (!name)" here.

> +               name = parse_events__term_type_str(term->type_term);
>
>         list_for_each_entry(t, &head_terms->terms, list) {
> +               const char *t_name = t->config;
> +
> +               if (!t_name)
> +                       t_name = parse_events__term_type_str(t->type_term);
> +
>                 if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM &&
> -                   t->config && !strcmp(t->config, term->config)) {
> +                   t_name && name && !strcmp(t_name, name)) {
>                         t->used = true;
>                         *value = t->val.num;
>                         return 0;
> @@ -1453,7 +1462,7 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
>         }
>
>         if (verbose > 0)
> -               printf("Required parameter '%s' not specified\n", term->config);
> +               printf("Required parameter '%s' not specified\n", name);
>
>         return -1;
>  }
> @@ -1494,6 +1503,7 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>         struct perf_pmu_format *format;
>         __u64 *vp;
>         __u64 val, max_val;
> +       const char *name;
>
>         /*
>          * If this is a parameter we've already used for parameterized-eval,
> @@ -1507,7 +1517,8 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>          * traditionally have had to handle not having a PMU. An alias may
>          * have hard coded config values, optionally apply them below.
>          */
> -       if (parse_events__is_hardcoded_term(term)) {
> +       if (parse_events__is_hardcoded_term(term) &&
> +                      term->type_val != PARSE_EVENTS__TERM_TYPE_STR) {

What is being guarded against in this case? Could you update or add a comment.

>                 /* Config terms set all bits in the config. */
>                 DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
>
> @@ -1576,7 +1587,11 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>                 return 0;
>         }
>
> -       format = pmu_find_format(&pmu->format, term->config);
> +       name = term->config;
> +       if (!name)
> +               name = parse_events__term_type_str(term->type_term);
> +
> +       format = pmu_find_format(&pmu->format, name);
>         if (!format) {
>                 char *pmu_term = pmu_formats_string(&pmu->format);
>                 char *unknown_term;
> @@ -2117,7 +2132,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, "=?")) {

I wonder if it would be safer if we did a `str_ends_with(alias->terms,
"=?")`. This likely applies to the other strstr usage.

Thanks,
Ian

>                 /* No parameters. */
>                 snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
>                 return buf;
> @@ -2129,15 +2144,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
>
>
>
> Thanks
> Athira
>
> >>
> >> Does this cause a NULL pointer dereference in the term iteration loop?
> >>
> >> list_for_each_entry(term, &terms.terms, list) {
> >> if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> >>   used += snprintf(buf + used, sub_non_neg(len, used),
> >>     ",%s=%s", term->config,
> >>     term->val.str);
> >> }
> >>
> >> Passing a NULL pointer to the %s format specifier for term->config might
> >> cause a segmentation fault or formatting corruption.
> >>
> >> Can this also lead to a stack buffer overflow due to how snprintf() return
> >> values are accumulated?
> >>
> >> snprintf() returns the number of characters that would have been written.
> >> If the event names and terms are long enough, the "used" variable can exceed
> >> "len" (which is typically 1024 bytes).
> >>
> >> When evaluating "buf + used" in the same loop:
> >>
> >> used += snprintf(buf + used, sub_non_neg(len, used), ...);
> >>
> >> If "used" exceeds 1024, this creates an out-of-bounds pointer. Modern
> >> compilers can use this undefined behavior to infer that "used" <= 1024
> >> must be true, which optimizes out the safety check inside sub_non_neg():
> >>
> >> static int sub_non_neg(int a, int b)
> >> {
> >> if (b > a)
> >>   return 0;
> >> return a - b;
> >> }
> >>
> >> The compiler can reduce this to simply "a - b" (or "len - used").
> >> If "used" is actually greater than 1024, "len - used" evaluates to a
> >> negative integer. This negative value is implicitly cast to size_t for
> >> snprintf(), resulting in a massive size limit.
> >>
> >> Would this cause snprintf() to write past the end of the stack buffer
> >> without bounds checking? Using scnprintf() might prevent "used" from
> >> exceeding "len".
> >>
> >> Thanks,
> >> Ian
> >>
> >>>> ---
> >>>> 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
> >>>>
> >>>
> >>> Tested this patch, and its working as expected.
> >>>
> >>> Before Patch:
> >>>
> >>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
> >>> hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]
> >>>
> >>> After Patch:
> >>>
> >>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
> >>> hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]
> >>>
> >>>
> >>> ./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/
> >>>
> >>>
> >>> Performance counter stats for 'system wide':
> >>>
> >>>       2018866563      hv_24x7/PM_PAU_CYC,chip=0/
> >>>
> >>>    229.938231314 seconds time elapsed
> >>>
> >>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> >>>
> >>> Regards,
> >>> Venkat.
>
>

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

[Athira Rajeev]

> On 24 Mar 2026, at 12:17 AM, Ian Rogers <irogers@google.com> wrote:
> 
> On Mon, Mar 23, 2026 at 5:19 AM Athira Rajeev <atrajeev@linux.ibm.com> wrote:
>> 
>> 
>> 
>>> On 18 Mar 2026, at 12:05 PM, Athira Rajeev <atrajeev@linux.ibm.com> wrote:
>>> 
>>> 
>>> 
>>>> On 17 Mar 2026, at 9:39 PM, Ian Rogers <irogers@google.com> wrote:
>>>> 
>>>> On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88@linux.ibm.com> wrote:
>>>>> 
>>>>>> On 14 Mar 2026, at 2:03 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.
>>>>>> 
>>>>>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
>>>> 
>>>> Thanks Athira, Sashiko is noting in its review:
>>>> https://sashiko.dev/#/patchset/20260314083304.75321-1-atrajeev%40linux.ibm.com
>>> 
>>> Thanks Ian for pointing this. Its interesting to see this review.
>>> I will check through the review.
>>> 
>>> Thanks
>>> Athira
>>>> 
>>>> By inverting this check, parameterized events now proceed to
>>>> parse_events_terms() and the rest of format_alias().
>>>> 
>>>> If a parameterized event uses a built-in perf keyword for its parameter name
>>>> (e.g., config=?), the lexer parses it as a predefined term token, which sets
>>>> term->config to NULL.
>> 
>> Hi All,
>> 
>> sashiko brought up a scenario :
>> 
>> “If a parameterized event uses a built-in perf keyword for its parameter name
>> (e.g., config=?), the lexer parses it as a predefined term token, which sets
>> term->config to NULL."
>> 
>> Existing usage for parameterized events in perf:
>> ======================================
>> 
>> In perf,  “?” is often used in event aliases to indicate that a parameter needs to be provided
>> by the user (e.g., param1=?). This information is exposed via “format” in sysfs too.  The current way,
>> how this is consumed in different PMU drivers is: example:
>> 
>>  # cat /sys/devices/pmu/format/param1
>>    config:12-15
>> 
>>   # cat /sys/devices/pmu/format/param2
>>    config1:0-17
>> 
>> Here param1 uses bits 12:15 in config and param2 used bits 0:17 in config1
>> 
>> And event will use these parameterized fields:
>>    #cat /sys/devices/pmu/events/event1       domain=1,param1=?,param2=?
>> 
>> “Perf list” expects "param1=?,param2=?” In the event listing
>>   # perf list|grep event1
>>   pmu/event1,param1=?,param2=?/                       [Kernel PMU event]
>> 
>> The patch here address fix in “strstr” check in format_alias for “perf list” to display these user expected “?” fields
>> in event alias.  Traditionally, perf treats config, config1, and config2 etc as hardcoded numeric
>> keywords or builtin keyword.
>> 
>> The scneario sashiko brought up is interesting. Where a parameterized event
>> uses a built-in perf keyword  for its parameter name.
>> 
>> sashiko review : Scenario where parameterized event uses a built-in perf keyword
>> ============================================================
>> Example, having “config2” in sysfs format-driven logic. Below is an example case.
>> 
>>    #cat /sys/devices/pmu/events/event1
>>     config2=?,domain=1,param1=?,param2=?
>> 
>>      # cat /sys/devices/hv_24x7/format/config2
>>         config2:0-16
>> 
>> Here “perf list” will show as (null) or might crash too as sashiko review pointed out.
>> 
>>    # perf list|grep event1
>>    pmu/event1,(null)=?,param1=?,param2=?/                    [Kernel PMU event]
>> 
>> This is because term->config is null for built-in keywords. perf lexer sets this to NULL
>> for predefined keywords ( like 'config', 'period'), storing the type in the enum instead.
>> 
>> To fix displaying the “=?” in “perf list” for keywords like config2,  format_alias() function needs to handle
>> null term->config , ie by using parse_events__term_type_str() to map NULL config
>> pointers back to their string keywords
>> 
>> But this only will fix the “perf list” part of it.  “Perf stat” or record will still fail since
>> currently these keywords are not considered as placeholders for “?” values.
>> 
>> If a PMU wants to use any builtin-keywords in parameters and want to ensure
>> it is set by user, this could be useful.  Again considering this as a use case, we need other changes
>> to allow these built-in keywords to function as placeholders (e.g., config2=?) within a PMU alias.
>> 
>> I am adding changes which I have identified needed for this support.
>> If it is better to post this separately as patch set and discuss, definitely will do that.
>> I wanted to run the idea of this new scenario with everyone. Apologies for the long text.
>> 
>> Thanks and looking for comments.
>> 
>> 
>> From 21179a76be97ef81c69a9492f0f432dec5b5361c Mon Sep 17 00:00:00 2001
>> From: Athira Rajeev <atrajeev@linux.ibm.com>
>> Date: Mon, 23 Mar 2026 08:38:30 -0400
>> Subject: [PATCH] tools/perf: Enable parameterized values for keywords like
>> config1
>> 
>> PMU defines events that require user-supplied parameters (like chip
>> or core) are packed into specific bit-fields of the different config
>> registers. Traditionally, perf treats config, config1, and config2
>> as hardcoded numeric keywords, causing it to bypass the sysfs
>> format-driven bit-masking logic.
>> 
>> This patch allows these built-in keywords to function as placeholders
>> (e.g., config2=?) within a PMU alias.
>> 
>> Changes includes:
>> - Updated config_term_pmu in parse-events.c to recognize string
>>  placeholders for event terms. otherwise it will expect those to
>>  be of type PARSE_EVENTS__TERM_TYPE_NUM
>> - Updated pmu_config_term in pmu.c to make sure if keywords are used
>>  as placeholders , they pass through sysfs based format logic
>> (pmu_find_format) and pmu_resolve_param_term() logic
>> - Modified format_alias() to ensure "perf list" display the parameterized
>>  events correctly
>> - Replace snprintf with scnprintf in buffer offset calculations to
>>   ensure the 'used' count will not exceed the "len"
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
>> ---
>> tools/perf/util/parse-events.c |  5 +++++
>> tools/perf/util/pmu.c          | 37 +++++++++++++++++++++++++---------
>> 2 files changed, 33 insertions(+), 9 deletions(-)
>> 
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index b9efb296bba5..282b0a0a5340 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1051,6 +1051,11 @@ static int config_term_pmu(struct perf_event_attr *attr,
>>                 */
>>                return 0;
>>        }
>> +
>> +       if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR && term->val.str)
>> +               if (strstr(term->val.str, "?"))
>> +                       return 0;
> 
> nit: Since the nested if spans more than two lines, could you add
> curly braces {} to the outer if statement. Could you also add a
> comment explaining why the runtime parameter causes an early return
> and why you used `strstr` instead of `strcmp`?


Hi Ian

Thanks for checking and responding with review comments.
Sure I will handle this.
> 
>> +
>>        return config_term_common(attr, term, parse_state);
>> }
>> 
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 23337d2fa281..2c4503e1a11b 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -1442,10 +1442,19 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
>>                                  __u64 *value)
>> {
>>        struct parse_events_term *t;
>> +       const char *name = term->config;
>> +
>> +       if (!term->config)
> 
> I think you meant "if (!name)" here.
Yes, will correct this 
> 
>> +               name = parse_events__term_type_str(term->type_term);
>> 
>>        list_for_each_entry(t, &head_terms->terms, list) {
>> +               const char *t_name = t->config;
>> +
>> +               if (!t_name)
>> +                       t_name = parse_events__term_type_str(t->type_term);
>> +
>>                if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM &&
>> -                   t->config && !strcmp(t->config, term->config)) {
>> +                   t_name && name && !strcmp(t_name, name)) {
>>                        t->used = true;
>>                        *value = t->val.num;
>>                        return 0;
>> @@ -1453,7 +1462,7 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
>>        }
>> 
>>        if (verbose > 0)
>> -               printf("Required parameter '%s' not specified\n", term->config);
>> +               printf("Required parameter '%s' not specified\n", name);
>> 
>>        return -1;
>> }
>> @@ -1494,6 +1503,7 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>>        struct perf_pmu_format *format;
>>        __u64 *vp;
>>        __u64 val, max_val;
>> +       const char *name;
>> 
>>        /*
>>         * If this is a parameter we've already used for parameterized-eval,
>> @@ -1507,7 +1517,8 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>>         * traditionally have had to handle not having a PMU. An alias may
>>         * have hard coded config values, optionally apply them below.
>>         */
>> -       if (parse_events__is_hardcoded_term(term)) {
>> +       if (parse_events__is_hardcoded_term(term) &&
>> +                      term->type_val != PARSE_EVENTS__TERM_TYPE_STR) {
> 
> What is being guarded against in this case? Could you update or add a comment.

Sure
> 
>>                /* Config terms set all bits in the config. */
>>                DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
>> 
>> @@ -1576,7 +1587,11 @@ static int pmu_config_term(const struct perf_pmu *pmu,
>>                return 0;
>>        }
>> 
>> -       format = pmu_find_format(&pmu->format, term->config);
>> +       name = term->config;
>> +       if (!name)
>> +               name = parse_events__term_type_str(term->type_term);
>> +
>> +       format = pmu_find_format(&pmu->format, name);
>>        if (!format) {
>>                char *pmu_term = pmu_formats_string(&pmu->format);
>>                char *unknown_term;
>> @@ -2117,7 +2132,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, "=?")) {
> 
> I wonder if it would be safer if we did a `str_ends_with(alias->terms,
> "=?")`. This likely applies to the other strstr usage.
Ok , 

I will post a V2 patchset with all changes, splitting the fix patch which address the “str” check above
and changes to allow using the builtin-keywords as place holders within PMU event alias

Thanks
Athira
> 
> Thanks,
> Ian
> 
>>                /* No parameters. */
>>                snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
>>                return buf;
>> @@ -2129,15 +2144,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
>> 
>> 
>> 
>> Thanks
>> Athira
>> 
>>>> 
>>>> Does this cause a NULL pointer dereference in the term iteration loop?
>>>> 
>>>> list_for_each_entry(term, &terms.terms, list) {
>>>> if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
>>>>  used += snprintf(buf + used, sub_non_neg(len, used),
>>>>    ",%s=%s", term->config,
>>>>    term->val.str);
>>>> }
>>>> 
>>>> Passing a NULL pointer to the %s format specifier for term->config might
>>>> cause a segmentation fault or formatting corruption.
>>>> 
>>>> Can this also lead to a stack buffer overflow due to how snprintf() return
>>>> values are accumulated?
>>>> 
>>>> snprintf() returns the number of characters that would have been written.
>>>> If the event names and terms are long enough, the "used" variable can exceed
>>>> "len" (which is typically 1024 bytes).
>>>> 
>>>> When evaluating "buf + used" in the same loop:
>>>> 
>>>> used += snprintf(buf + used, sub_non_neg(len, used), ...);
>>>> 
>>>> If "used" exceeds 1024, this creates an out-of-bounds pointer. Modern
>>>> compilers can use this undefined behavior to infer that "used" <= 1024
>>>> must be true, which optimizes out the safety check inside sub_non_neg():
>>>> 
>>>> static int sub_non_neg(int a, int b)
>>>> {
>>>> if (b > a)
>>>>  return 0;
>>>> return a - b;
>>>> }
>>>> 
>>>> The compiler can reduce this to simply "a - b" (or "len - used").
>>>> If "used" is actually greater than 1024, "len - used" evaluates to a
>>>> negative integer. This negative value is implicitly cast to size_t for
>>>> snprintf(), resulting in a massive size limit.
>>>> 
>>>> Would this cause snprintf() to write past the end of the stack buffer
>>>> without bounds checking? Using scnprintf() might prevent "used" from
>>>> exceeding "len".
>>>> 
>>>> Thanks,
>>>> Ian
>>>> 
>>>>>> ---
>>>>>> 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
>>>>>> 
>>>>> 
>>>>> Tested this patch, and its working as expected.
>>>>> 
>>>>> Before Patch:
>>>>> 
>>>>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>>>>> hv_24x7/CPM_EXT_INT_OS/                            [Kernel PMU event]
>>>>> 
>>>>> After Patch:
>>>>> 
>>>>> ./perf list hv_24x7 | grep -i CPM_EXT_INT_OS
>>>>> hv_24x7/CPM_EXT_INT_OS,domain=?,core=?/ [Kernel PMU event]
>>>>> 
>>>>> 
>>>>> ./perf stat -e hv_24x7/PM_PAU_CYC,chip=0/
>>>>> 
>>>>> 
>>>>> Performance counter stats for 'system wide':
>>>>> 
>>>>>      2018866563      hv_24x7/PM_PAU_CYC,chip=0/
>>>>> 
>>>>>   229.938231314 seconds time elapsed
>>>>> 
>>>>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>>>>> 
>>>>> Regards,
>>>>> Venkat.