> On 17 Mar 2026, at 9:39 PM, Ian Rogers <[email protected]> wrote:
>
> On Tue, Mar 17, 2026 at 1:56 AM Venkat <[email protected]> wrote:
>>
>>> On 14 Mar 2026, at 2:03 PM, Athira Rajeev <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
>>
>> Regards,
>> Venkat.
