[PATCH] tools/perf: Fix the check for parameterized field in event term
Athira Rajeev
atrajeev at linux.ibm.com
Mon Mar 23 23:18:49 AEDT 2026
> On 18 Mar 2026, at 12:05 PM, Athira Rajeev <atrajeev at linux.ibm.com> wrote:
>
>
>
>> On 17 Mar 2026, at 9:39 PM, Ian Rogers <irogers at google.com> wrote:
>>
>> On Tue, Mar 17, 2026 at 1:56 AM Venkat <venkat88 at linux.ibm.com> wrote:
>>>
>>>> On 14 Mar 2026, at 2:03 PM, Athira Rajeev <atrajeev at 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 at 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 at 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 at 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 at linux.ibm.com>
>>>
>>> Regards,
>>> Venkat.
More information about the Linuxppc-dev
mailing list