perf list dumps core using 5.13.0.rc2

perf list dumps core using 5.13.0.rc2

[Thomas Richter]

Using kernel 5.13.0.rc2 command perf list dumps core on my x86
virtual machine:

[root@f34 perf]# ./perf list
Segmentation fault (core dumped)
^C
[root@f34 perf]#

The root case this this change in file ../include/uapi/linux/perf_event.h:
enum perf_sw_ids {
        PERF_COUNT_SW_CPU_CLOCK                 = 0,
        ...
 --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,

        PERF_COUNT_SW_MAX,                      /* non-ABI */
};

This change increases PERF_COUNT_SW_MAX to 12. However this
change is not reflected in file util/parse-events.c where data structure

struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
        ....
}

is defined and it misses the symbol name and alias for this new software
event. So when command 'perf list' is called, the call chain sequence
is
  cmd_list()
    print_events()
      print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
                          event_symbols_sw, PERF_COUNT_SW_MAX, name_only);

where PERF_COUNT_SW_MAX is 12 and  structure event_symbols_sw[] contains
only 11 elements. This ends up with the last element of the array being
all zeroes and the line:

    if (!name_only && strlen(syms->alias))

dumps core because syms->alias is a NULL pointer.

Is this dummy event PERF_COUNT_SW_CGROUP_SWITCHES simply missing in the
event_symbols_sw[] array or is there more to it (which I am missing).

Thanks. 

PS: I can sent a patch if needed....

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: perf list dumps core using 5.13.0.rc2

[Arnaldo Carvalho de Melo]

Em Wed, May 19, 2021 at 04:57:05PM +0200, Thomas Richter escreveu:
> Using kernel 5.13.0.rc2 command perf list dumps core on my x86
> virtual machine:
> 
> [root@f34 perf]# ./perf list
> Segmentation fault (core dumped)
> ^C
> [root@f34 perf]#
> 
> The root case this this change in file ../include/uapi/linux/perf_event.h:
> enum perf_sw_ids {
>         PERF_COUNT_SW_CPU_CLOCK                 = 0,
>         ...
>  --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,
> 
>         PERF_COUNT_SW_MAX,                      /* non-ABI */
> };
> 
> This change increases PERF_COUNT_SW_MAX to 12. However this
> change is not reflected in file util/parse-events.c where data structure
> 
> struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
>         ....
> }
> 
> is defined and it misses the symbol name and alias for this new software

Sure, it is not there, but then PERF_COUNT_SW_MAX is used in the array
definition, so it ends up having
event_symbols_sw[PERF_COUNT_SW_MAX].symbol == NULL

> event. So when command 'perf list' is called, the call chain sequence
> is
>   cmd_list()
>     print_events()
>       print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
>                           event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
> 
> where PERF_COUNT_SW_MAX is 12 and  structure event_symbols_sw[] contains
> only 11 elements. This ends up with the last element of the array being
> all zeroes and the line:
> 
>     if (!name_only && strlen(syms->alias))

But before this we have:

                if (event_glob != NULL && syms->symbol != NULL &&
                    !(strglobmatch(syms->symbol, event_glob) ||
                      (syms->alias && strglobmatch(syms->alias, event_glob))))
                        continue;

We need to move that syms->symbol != NULL part to be the first one and
continue if that array entry isn't populated, I'll add a patch with your
Reported-by:

I was scratching my head looking why I hadn't hit this, it doesn't
support PERF_COUNT_SW_CGROUP_SWITCHES so it bails out at
!is_event_supported(type, i):

        for (i = 0; i < max; i++, syms++) {

                if (event_glob != NULL && syms->symbol != NULL &&
                    !(strglobmatch(syms->symbol, event_glob) ||
                      (syms->alias && strglobmatch(syms->alias, event_glob))))
                        continue;

                if (!is_event_supported(type, i))
                        continue;

                if (!evt_num_known) {
                        evt_num++;
                        continue;
                }

                if (!name_only && strlen(syms->alias))

> 
> dumps core because syms->alias is a NULL pointer.
> 
> Is this dummy event PERF_COUNT_SW_CGROUP_SWITCHES simply missing in the
> event_symbols_sw[] array or is there more to it (which I am missing).
> 
> Thanks. 
> 
> PS: I can sent a patch if needed....
> 
> -- 
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> Vorsitzender des Aufsichtsrats: Gregor Pillen
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

-- 

- Arnaldo

Re: perf list dumps core using 5.13.0.rc2

[Ian Rogers]

On Wed, May 19, 2021 at 7:57 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> Using kernel 5.13.0.rc2 command perf list dumps core on my x86
> virtual machine:
>
> [root@f34 perf]# ./perf list
> Segmentation fault (core dumped)
> ^C
> [root@f34 perf]#
>
> The root case this this change in file ../include/uapi/linux/perf_event.h:
> enum perf_sw_ids {
>         PERF_COUNT_SW_CPU_CLOCK                 = 0,
>         ...
>  --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,
>
>         PERF_COUNT_SW_MAX,                      /* non-ABI */
> };
>
> This change increases PERF_COUNT_SW_MAX to 12. However this
> change is not reflected in file util/parse-events.c where data structure
>
> struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
>         ....
> }
>
> is defined and it misses the symbol name and alias for this new software
> event. So when command 'perf list' is called, the call chain sequence
> is
>   cmd_list()
>     print_events()
>       print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
>                           event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
>
> where PERF_COUNT_SW_MAX is 12 and  structure event_symbols_sw[] contains
> only 11 elements. This ends up with the last element of the array being
> all zeroes and the line:
>
>     if (!name_only && strlen(syms->alias))
>
> dumps core because syms->alias is a NULL pointer.
>
> Is this dummy event PERF_COUNT_SW_CGROUP_SWITCHES simply missing in the
> event_symbols_sw[] array or is there more to it (which I am missing).
>
> Thanks.
>
> PS: I can sent a patch if needed....

I believe Namhyung's pending change will fix this:
https://lore.kernel.org/lkml/20210210083327.22726-3-namhyung@kernel.org/

Thanks,
Ian

> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> Vorsitzender des Aufsichtsrats: Gregor Pillen
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: perf list dumps core using 5.13.0.rc2

[Arnaldo Carvalho de Melo]

Em Wed, May 19, 2021 at 01:26:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, May 19, 2021 at 04:57:05PM +0200, Thomas Richter escreveu:
> > Using kernel 5.13.0.rc2 command perf list dumps core on my x86
> > virtual machine:
> > 
> > [root@f34 perf]# ./perf list
> > Segmentation fault (core dumped)
> > ^C
> > [root@f34 perf]#
> > 
> > The root case this this change in file ../include/uapi/linux/perf_event.h:
> > enum perf_sw_ids {
> >         PERF_COUNT_SW_CPU_CLOCK                 = 0,
> >         ...
> >  --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,
> > 
> >         PERF_COUNT_SW_MAX,                      /* non-ABI */
> > };
> > 
> > This change increases PERF_COUNT_SW_MAX to 12. However this
> > change is not reflected in file util/parse-events.c where data structure
> > 
> > struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
> >         ....
> > }
> > 
> > is defined and it misses the symbol name and alias for this new software

Please check and provide your Acked-by or Reviewed-by,

Thanks,

- Arnaldo

From f8b0fe090267a901aa30070351b40f5e8abc0286 Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Wed, 19 May 2021 13:50:31 -0300
Subject: [PATCH 1/1] perf parse-events: Check if the software events array
 slots are populated

To avoid a NULL pointer dereference when the kernel supports the new
entry but the tooling still hasn't added an entry for it.

This happened with the recently added PERF_COUNT_SW_CGROUP_SWITCHES
software event.

Reported-by: Thomas Richter <tmricht@linux.ibm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Sumanth Korikkar <sumanthk@linux.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 4dad14265b81dfb8..8c5df56121d85cb2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2928,9 +2928,14 @@ void print_symbol_events(const char *event_glob, unsigned type,
 	}
 
 	for (i = 0; i < max; i++, syms++) {
+		/*
+		 * New attr.config still not supported here, the latest
+		 * example was PERF_COUNT_SW_CGROUP_SWITCHES
+		 */
+		if (syms->symbol == NULL)
+			continue;
 
-		if (event_glob != NULL && syms->symbol != NULL &&
-		    !(strglobmatch(syms->symbol, event_glob) ||
+		if (event_glob != NULL && !(strglobmatch(syms->symbol, event_glob) ||
 		      (syms->alias && strglobmatch(syms->alias, event_glob))))
 			continue;
 
-- 
2.31.1

Re: perf list dumps core using 5.13.0.rc2

[Arnaldo Carvalho de Melo]

Em Wed, May 19, 2021 at 09:27:14AM -0700, Ian Rogers escreveu:
> On Wed, May 19, 2021 at 7:57 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
> >
> > Using kernel 5.13.0.rc2 command perf list dumps core on my x86
> > virtual machine:
> >
> > [root@f34 perf]# ./perf list
> > Segmentation fault (core dumped)
> > ^C
> > [root@f34 perf]#
> >
> > The root case this this change in file ../include/uapi/linux/perf_event.h:
> > enum perf_sw_ids {
> >         PERF_COUNT_SW_CPU_CLOCK                 = 0,
> >         ...
> >  --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,
> >
> >         PERF_COUNT_SW_MAX,                      /* non-ABI */
> > };
> >
> > This change increases PERF_COUNT_SW_MAX to 12. However this
> > change is not reflected in file util/parse-events.c where data structure
> >
> > struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
> >         ....
> > }
> >
> > is defined and it misses the symbol name and alias for this new software
> > event. So when command 'perf list' is called, the call chain sequence
> > is
> >   cmd_list()
> >     print_events()
> >       print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
> >                           event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
> >
> > where PERF_COUNT_SW_MAX is 12 and  structure event_symbols_sw[] contains
> > only 11 elements. This ends up with the last element of the array being
> > all zeroes and the line:
> >
> >     if (!name_only && strlen(syms->alias))
> >
> > dumps core because syms->alias is a NULL pointer.
> >
> > Is this dummy event PERF_COUNT_SW_CGROUP_SWITCHES simply missing in the
> > event_symbols_sw[] array or is there more to it (which I am missing).
> >
> > Thanks.
> >
> > PS: I can sent a patch if needed....
> 
> I believe Namhyung's pending change will fix this:
> https://lore.kernel.org/lkml/20210210083327.22726-3-namhyung@kernel.org/

Ok, I'll pick that one instead of the one I did, similar one.

- Arnaldo

Re: perf list dumps core using 5.13.0.rc2

[Thomas Richter]

On 5/19/21 7:18 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 19, 2021 at 09:27:14AM -0700, Ian Rogers escreveu:
>> On Wed, May 19, 2021 at 7:57 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>>>
>>> Using kernel 5.13.0.rc2 command perf list dumps core on my x86
>>> virtual machine:
>>>
>>> [root@f34 perf]# ./perf list
>>> Segmentation fault (core dumped)
>>> ^C
>>> [root@f34 perf]#
>>>
>>> The root case this this change in file ../include/uapi/linux/perf_event.h:
>>> enum perf_sw_ids {
>>>         PERF_COUNT_SW_CPU_CLOCK                 = 0,
>>>         ...
>>>  --->   PERF_COUNT_SW_CGROUP_SWITCHES           = 11,
>>>
>>>         PERF_COUNT_SW_MAX,                      /* non-ABI */
>>> };
>>>
>>> This change increases PERF_COUNT_SW_MAX to 12. However this
>>> change is not reflected in file util/parse-events.c where data structure
>>>
>>> struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
>>>         ....
>>> }
>>>
>>> is defined and it misses the symbol name and alias for this new software
>>> event. So when command 'perf list' is called, the call chain sequence
>>> is
>>>   cmd_list()
>>>     print_events()
>>>       print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
>>>                           event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
>>>
>>> where PERF_COUNT_SW_MAX is 12 and  structure event_symbols_sw[] contains
>>> only 11 elements. This ends up with the last element of the array being
>>> all zeroes and the line:
>>>
>>>     if (!name_only && strlen(syms->alias))
>>>
>>> dumps core because syms->alias is a NULL pointer.
>>>
>>> Is this dummy event PERF_COUNT_SW_CGROUP_SWITCHES simply missing in the
>>> event_symbols_sw[] array or is there more to it (which I am missing).
>>>
>>> Thanks.
>>>
>>> PS: I can sent a patch if needed....
>>
>> I believe Namhyung's pending change will fix this:
>> https://lore.kernel.org/lkml/20210210083327.22726-3-namhyung@kernel.org/
> 
> Ok, I'll pick that one instead of the one I did, similar one.
> 
> - Arnaldo
> 

Thanks for looking into this, Namhyung's patch fixes this issue.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294