linux-kernel - Re: [PATCH] perf pmus: Fix duplicate events caused segfault

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: <2C7FF61F-2165-47D4-83A4-B0230D50844D@linux.vnet.ibm.com>
Date: Sat, 20 Jul 2024 13:38:29 +0530
From: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
To: Eric Lin <eric.lin@...ive.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>, Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        James Clark <james.clark@....com>,
        linux-perf-users <linux-perf-users@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, vincent.chen@...ive.com,
        greentime.hu@...ive.com
Subject: Re: [PATCH] perf pmus: Fix duplicate events caused segfault



> On 19 Jul 2024, at 1:46 PM, Eric Lin <eric.lin@...ive.com> wrote:
> 
> Currently, if vendor JSON files have two duplicate event names,
> the "perf list" command will trigger a segfault.
> 
> In commit e6ff1eed3584 ("perf pmu: Lazily add JSON events"),
> pmu_events_table__num_events() gets the number of JSON events
> from table_pmu->num_entries, which includes duplicate events
> if there are duplicate event names in the JSON files.

Hi Eric,

Let us consider there are duplicate event names in the JSON files, say :

metric.json with: EventName as pmu_cache_miss, EventCode as 0x1
cache.json with:  EventName as pmu_cache_miss, EventCode as 0x2

If we fix the segfault and proceed, still “perf list” will list only one entry for pmu_cache_miss with may be 0x1/0x2 as event code ?
Can you check the result to confirm what “perf list” will list in this case ? If it’s going to have only one entry in perf list, does it mean there are two event codes for pmu_cache_miss and it can work with either of the event code ?

If it happens to be a mistake in json file to have duplicate entry with different event code (ex: with some broken commit), I am thinking if the better fix is to keep only the valid entry in json file ?

Thanks
Athira

> 
> perf_pmu__for_each_event() adds JSON events to the pmu->alias
> list and copies sevent data to the aliases array. However, the
> pmu->alias list does not contain duplicate events, as
> perf_pmu__new_alias() checks if the name was already created.
> 
> When sorting the alias data, if there are two duplicate events,
> it causes a segfault in cmp_sevent() due to invalid aliases in
> the aliases array.
> 
> To avoid such segfault caused by duplicate event names in the
> JSON files, the len should be updated before sorting the aliases.
> 
> Fixes: e6ff1eed3584 ("perf pmu: Lazily add JSON events")
> Signed-off-by: Eric Lin <eric.lin@...ive.com>
> ---
> tools/perf/util/pmus.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index b9b4c5eb5002..e38c3fd4d1ff 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -443,7 +443,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> {
> struct perf_pmu *pmu;
> int printed = 0;
> - int len;
> + size_t len, j;
> struct sevent *aliases;
> struct events_callback_state state;
> bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
> @@ -474,8 +474,9 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
> perf_pmus__print_pmu_events__callback);
> }
> + len = state.index;
> qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
> - for (int j = 0; j < len; j++) {
> + for (j = 0; j < len; j++) {
> /* Skip duplicates */
> if (j > 0 && pmu_alias_is_duplicate(&aliases[j], &aliases[j - 1]))
> continue;
> -- 
> 2.43.2
> 
>