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

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

Reply via email to