linux-kernel - RE: [PATCH 2/2] perf pmu: Fix num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <DBBPR08MB4538A157B81F2BBD2DD9E40DF7E72@DBBPR08MB4538.eurprd08.prod.outlook.com>
Date: Fri, 10 May 2024 06:52:08 +0000
From: Justin He <Justin.He@....com>
To: Ian Rogers <irogers@...gle.com>
CC: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim
	<namhyung@...nel.org>, Mark Rutland <Mark.Rutland@....com>, Alexander
 Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang
	<kan.liang@...ux.intel.com>, James Clark <James.Clark@....com>,
	"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, nd
	<nd@....com>
Subject: RE: [PATCH 2/2] perf pmu: Fix num_events calculation

Hi, Ian

> -----Original Message-----
> From: Ian Rogers <irogers@...gle.com>
> Sent: Friday, May 10, 2024 2:17 PM
> To: Justin He <Justin.He@....com>
> Cc: Peter Zijlstra <peterz@...radead.org>; Ingo Molnar <mingo@...hat.com>;
> Arnaldo Carvalho de Melo <acme@...nel.org>; Namhyung Kim
> <namhyung@...nel.org>; Mark Rutland <Mark.Rutland@....com>; Alexander
> Shishkin <alexander.shishkin@...ux.intel.com>; Jiri Olsa <jolsa@...nel.org>;
> Adrian Hunter <adrian.hunter@...el.com>; Kan Liang
> <kan.liang@...ux.intel.com>; James Clark <James.Clark@....com>;
> linux-perf-users@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH 2/2] perf pmu: Fix num_events calculation
> 
> On Thu, May 9, 2024 at 7:47 PM Jia He <justin.he@....com> wrote:
> >
> > When pe is NULL in the function perf_pmu__new_alias(), the total
> > number of events is added to loaded_json_aliases. However, if
> > pmu->events_table is NULL and cpu_aliases_added is false, the
> > calculation for the events number in perf_pmu__num_events() is incorrect.
> >
> > Then cause the error report after "perf list":
> > Unexpected event
> smmuv3_pmcg_3f062/smmuv3_pmcg_3f062/transaction//
> >
> > Fix it by adding loaded_json_aliases in the calculation under the
> > mentioned conditions.
> >
> > Test it also with "perf bench internals pmu-scan" and there is no
> > regression.
> >
> > Fixes: e6ff1eed3584 ("perf pmu: Lazily add JSON events")
> > Signed-off-by: Jia He <justin.he@....com>
> > ---
> >  tools/perf/util/pmu.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index
> > a1eef7b2e389..a53224e2ce7e 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1639,6 +1639,8 @@ size_t perf_pmu__num_events(struct perf_pmu
> *pmu)
> >                  nr += pmu->loaded_json_aliases;
> >         else if (pmu->events_table)
> >                 nr +=
> pmu_events_table__num_events(pmu->events_table,
> > pmu) - pmu->loaded_json_aliases;
> > +       else
> > +               nr += pmu->loaded_json_aliases;
> 
> Thanks for working on this! The "struct pmu_event *pe" in new_alias is an entry
> from the json data, and "pmu->events_table" should NULL if there is no json
> data. I believe the code is assuming that these lines aren't necessary as it
> shouldn't be possible to load json data if the json events table doesn't exist for
> the PMU - if there is no json data then loaded_json_aliases should be 0 and no
> addition is necessary. I'm wondering why this case isn't true for you.
On my Armv8a N2 server, "pmu->events_table" is NULL because perf_pmu__find_events_table()
return NULL.

I also noticed that pmu->loaded_json_aliases is *not* 0. The missing adding calculation will cause
perf_pmu__num_events() less than normal case and will trigger latter check failure in
perf_pmus__print_pmu_events__callback().
At last, perf list will report many lines similar as:
Unexpected event smmuv3_pmcg_3f062/smmuv3_pmcg_3f062/transaction//


--
Cheers,
Justin (Jia He)