From: He Zhe <[email protected]>
NULL pointer may be passed to perf_cpu_map__cpu and then cause the
following crash.
perf ftrace -G start_kernel ls
failed to set tracing filters
[ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
[ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
4d f4 31 c9 8b 45 08 8b9
Segmentation fault
Program received signal SIGSEGV, Segmentation fault.
0x5677dc98 in perf_cpu_map__cpu (cpus=0x0, idx=0) at cpumap.c:250
250 cpumap.c: No such file or directory.
(gdb) bt
0 0x5677dc98 in perf_cpu_map__cpu (cpus=0x0, idx=0) at cpumap.c:250
1 0x566790bd in evlist__close (evlist=0x56a6f470) at util/evlist.c:1222
2 0x566792aa in evlist__delete (evlist=evlist@entry=0x56a6f470)
at util/evlist.c:152
3 0x5667936b in evlist__delete (evlist=0x56a6f470) at util/evlist.c:148
4 0x565efd39 in cmd_ftrace (argc=1, argv=0xffffdd18) at builtin-ftrace.c:520
5 0x56660ee7 in run_builtin (p=0x56993004 <commands+324>, argc=4,
argv=0xffffdd18) at perf.c:312
6 0x565e7fae in handle_internal_command (argv=<optimized out>,
argc=<optimized out>) at perf.c:364
7 run_argv (argcp=<optimized out>, argv=<optimized out>) at perf.c:408
8 main (argc=<optimized out>, argv=<optimized out>) at perf.c:538
Add null pointer check for iteration and NULL assignment for all_cpus.
And there is no need to iterate if there is no cpus.
Signed-off-by: He Zhe <[email protected]>
---
tools/lib/perf/cpumap.c | 4 ++--
tools/lib/perf/evlist.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
index f93f4e703e4c..128386647ac0 100644
--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -247,7 +247,7 @@ struct perf_cpu_map *perf_cpu_map__new(const char *cpu_list)
int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx)
{
- if (idx < cpus->nr)
+ if (cpus && idx < cpus->nr)
return cpus->map[idx];
return -1;
@@ -255,7 +255,7 @@ int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx)
int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
- return cpus ? cpus->nr : 1;
+ return cpus ? cpus->nr : 0;
}
bool perf_cpu_map__empty(const struct perf_cpu_map *map)
diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index ae9e65aa2491..d57adf3020fe 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -127,6 +127,7 @@ void perf_evlist__exit(struct perf_evlist *evlist)
perf_cpu_map__put(evlist->cpus);
perf_thread_map__put(evlist->threads);
evlist->cpus = NULL;
+ evlist->all_cpus = NULL;
evlist->threads = NULL;
fdarray__exit(&evlist->pollfd);
}
--
2.24.1
Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
> From: He Zhe <[email protected]>
>
> NULL pointer may be passed to perf_cpu_map__cpu and then cause the
> following crash.
>
> perf ftrace -G start_kernel ls
> failed to set tracing filters
> [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
> sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
> [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
> 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
> 4d f4 31 c9 8b 45 08 8b9
> Segmentation fault
I'm not being able to repro this here, what is the tree you are using?
- Arnaldo
> Program received signal SIGSEGV, Segmentation fault.
> 0x5677dc98 in perf_cpu_map__cpu (cpus=0x0, idx=0) at cpumap.c:250
> 250 cpumap.c: No such file or directory.
> (gdb) bt
> 0 0x5677dc98 in perf_cpu_map__cpu (cpus=0x0, idx=0) at cpumap.c:250
> 1 0x566790bd in evlist__close (evlist=0x56a6f470) at util/evlist.c:1222
> 2 0x566792aa in evlist__delete (evlist=evlist@entry=0x56a6f470)
> at util/evlist.c:152
> 3 0x5667936b in evlist__delete (evlist=0x56a6f470) at util/evlist.c:148
> 4 0x565efd39 in cmd_ftrace (argc=1, argv=0xffffdd18) at builtin-ftrace.c:520
> 5 0x56660ee7 in run_builtin (p=0x56993004 <commands+324>, argc=4,
> argv=0xffffdd18) at perf.c:312
> 6 0x565e7fae in handle_internal_command (argv=<optimized out>,
> argc=<optimized out>) at perf.c:364
> 7 run_argv (argcp=<optimized out>, argv=<optimized out>) at perf.c:408
> 8 main (argc=<optimized out>, argv=<optimized out>) at perf.c:538
>
> Add null pointer check for iteration and NULL assignment for all_cpus.
> And there is no need to iterate if there is no cpus.
>
> Signed-off-by: He Zhe <[email protected]>
> ---
> tools/lib/perf/cpumap.c | 4 ++--
> tools/lib/perf/evlist.c | 1 +
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> index f93f4e703e4c..128386647ac0 100644
> --- a/tools/lib/perf/cpumap.c
> +++ b/tools/lib/perf/cpumap.c
> @@ -247,7 +247,7 @@ struct perf_cpu_map *perf_cpu_map__new(const char *cpu_list)
>
> int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx)
> {
> - if (idx < cpus->nr)
> + if (cpus && idx < cpus->nr)
> return cpus->map[idx];
>
> return -1;
> @@ -255,7 +255,7 @@ int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx)
>
> int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
> {
> - return cpus ? cpus->nr : 1;
> + return cpus ? cpus->nr : 0;
> }
>
> bool perf_cpu_map__empty(const struct perf_cpu_map *map)
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index ae9e65aa2491..d57adf3020fe 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -127,6 +127,7 @@ void perf_evlist__exit(struct perf_evlist *evlist)
> perf_cpu_map__put(evlist->cpus);
> perf_thread_map__put(evlist->threads);
> evlist->cpus = NULL;
> + evlist->all_cpus = NULL;
> evlist->threads = NULL;
> fdarray__exit(&evlist->pollfd);
> }
> --
> 2.24.1
On Thu, Mar 05, 2020 at 12:27:55PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
> > From: He Zhe <[email protected]>
> >
> > NULL pointer may be passed to perf_cpu_map__cpu and then cause the
> > following crash.
> >
> > perf ftrace -G start_kernel ls
> > failed to set tracing filters
> > [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
> > sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
> > [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
> > 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
> > 4d f4 31 c9 8b 45 08 8b9
> > Segmentation fault
>
> I'm not being able to repro this here, what is the tree you are using?
I believe that's the same bug that Jann Horn reported recently for perf trace.
I thought the patch for that went in.
-Andi
Em Thu, Mar 05, 2020 at 10:32:06AM -0800, Andi Kleen escreveu:
> On Thu, Mar 05, 2020 at 12:27:55PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
> > > From: He Zhe <[email protected]>
> > >
> > > NULL pointer may be passed to perf_cpu_map__cpu and then cause the
> > > following crash.
> > >
> > > perf ftrace -G start_kernel ls
> > > failed to set tracing filters
> > > [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
> > > sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
> > > [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
> > > 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
> > > 4d f4 31 c9 8b 45 08 8b9
> > > Segmentation fault
> >
> > I'm not being able to repro this here, what is the tree you are using?
>
> I believe that's the same bug that Jann Horn reported recently for perf trace.
> I thought the patch for that went in.
Ok, Zhe, that patch is at the end of this message, and it is in:
[acme@five perf]$ git tag --contains cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1 | grep ^v
v5.6-rc1
v5.6-rc2
v5.6-rc3
v5.6-rc4
[acme@five perf]$
Can you try with that?
- Arnaldo
commit cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1
Author: Jiri Olsa <[email protected]>
Date: Fri Jan 10 16:15:37 2020 +0100
libperf: Setup initial evlist::all_cpus value
Jann Horn reported crash in perf ftrace because evlist::all_cpus isn't
initialized if there's evlist without events, which is the case for perf
ftrace.
Adding initial initialization of evlist::all_cpus from given cpus,
regardless of events in the evlist.
Fixes: 7736627b865d ("perf stat: Use affinity for closing file descriptors")
Reported-by: Jann Horn <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
Acked-by: Andi Kleen <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Michael Petlan <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index ae9e65aa2491..5b9f2ca50591 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -164,6 +164,9 @@ void perf_evlist__set_maps(struct perf_evlist *evlist,
evlist->threads = perf_thread_map__get(threads);
}
+ if (!evlist->all_cpus && cpus)
+ evlist->all_cpus = perf_cpu_map__get(cpus);
+
perf_evlist__propagate_maps(evlist);
}
On 3/6/20 3:58 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 05, 2020 at 10:32:06AM -0800, Andi Kleen escreveu:
>> On Thu, Mar 05, 2020 at 12:27:55PM -0300, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
>>>> From: He Zhe <[email protected]>
>>>>
>>>> NULL pointer may be passed to perf_cpu_map__cpu and then cause the
>>>> following crash.
>>>>
>>>> perf ftrace -G start_kernel ls
>>>> failed to set tracing filters
>>>> [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
>>>> sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
>>>> [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
>>>> 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
>>>> 4d f4 31 c9 8b 45 08 8b9
>>>> Segmentation fault
>>> I'm not being able to repro this here, what is the tree you are using?
>> I believe that's the same bug that Jann Horn reported recently for perf trace.
>> I thought the patch for that went in.
> Ok, Zhe, that patch is at the end of this message, and it is in:
>
> [acme@five perf]$ git tag --contains cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1 | grep ^v
> v5.6-rc1
> v5.6-rc2
> v5.6-rc3
> v5.6-rc4
> [acme@five perf]$
>
> Can you try with that?
Thanks, that does fix the issue I met.
BTW, my change in perf_cpu_map__cpu can be used as a preventive check
and the "1" in perf_cpu_map__cpu should be "0", and assigning a NULL in
perf_evlist__exit makes the clearing complete. So are they worth a new patch?
Regards,
Zhe
>
> - Arnaldo
>
> commit cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1
> Author: Jiri Olsa <[email protected]>
> Date: Fri Jan 10 16:15:37 2020 +0100
>
> libperf: Setup initial evlist::all_cpus value
>
> Jann Horn reported crash in perf ftrace because evlist::all_cpus isn't
> initialized if there's evlist without events, which is the case for perf
> ftrace.
>
> Adding initial initialization of evlist::all_cpus from given cpus,
> regardless of events in the evlist.
>
> Fixes: 7736627b865d ("perf stat: Use affinity for closing file descriptors")
> Reported-by: Jann Horn <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
> Acked-by: Andi Kleen <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Michael Petlan <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Link: http://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index ae9e65aa2491..5b9f2ca50591 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -164,6 +164,9 @@ void perf_evlist__set_maps(struct perf_evlist *evlist,
> evlist->threads = perf_thread_map__get(threads);
> }
>
> + if (!evlist->all_cpus && cpus)
> + evlist->all_cpus = perf_cpu_map__get(cpus);
> +
> perf_evlist__propagate_maps(evlist);
> }
>
>
On Fri, Mar 06, 2020 at 03:20:55PM +0800, He Zhe wrote:
>
>
> On 3/6/20 3:58 AM, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Mar 05, 2020 at 10:32:06AM -0800, Andi Kleen escreveu:
> >> On Thu, Mar 05, 2020 at 12:27:55PM -0300, Arnaldo Carvalho de Melo wrote:
> >>> Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
> >>>> From: He Zhe <[email protected]>
> >>>>
> >>>> NULL pointer may be passed to perf_cpu_map__cpu and then cause the
> >>>> following crash.
> >>>>
> >>>> perf ftrace -G start_kernel ls
> >>>> failed to set tracing filters
> >>>> [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
> >>>> sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
> >>>> [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
> >>>> 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
> >>>> 4d f4 31 c9 8b 45 08 8b9
> >>>> Segmentation fault
> >>> I'm not being able to repro this here, what is the tree you are using?
> >> I believe that's the same bug that Jann Horn reported recently for perf trace.
> >> I thought the patch for that went in.
> > Ok, Zhe, that patch is at the end of this message, and it is in:
> >
> > [acme@five perf]$ git tag --contains cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1 | grep ^v
> > v5.6-rc1
> > v5.6-rc2
> > v5.6-rc3
> > v5.6-rc4
> > [acme@five perf]$
> >
> > Can you try with that?
>
> Thanks, that does fix the issue I met.
>
> BTW, my change in perf_cpu_map__cpu can be used as a preventive check
> and the "1"? in perf_cpu_map__cpu should be "0", and assigning a NULL in
I agree, can't see why we had 1 in here.. must be connected to the dummy
map.. could you please double check with all the perf_cpu_map__nr usages
that the 0 will work as expected?
> perf_evlist__exit makes the clearing complete. So are they worth a new patch?
the rest of the hunks looks good as preventive checks
thanks,
jirka
On 3/6/20 4:30 PM, Jiri Olsa wrote:
> On Fri, Mar 06, 2020 at 03:20:55PM +0800, He Zhe wrote:
>>
>> On 3/6/20 3:58 AM, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Mar 05, 2020 at 10:32:06AM -0800, Andi Kleen escreveu:
>>>> On Thu, Mar 05, 2020 at 12:27:55PM -0300, Arnaldo Carvalho de Melo wrote:
>>>>> Em Thu, Mar 05, 2020 at 06:47:19PM +0800, [email protected] escreveu:
>>>>>> From: He Zhe <[email protected]>
>>>>>>
>>>>>> NULL pointer may be passed to perf_cpu_map__cpu and then cause the
>>>>>> following crash.
>>>>>>
>>>>>> perf ftrace -G start_kernel ls
>>>>>> failed to set tracing filters
>>>>>> [ 208.710716] perf[341]: segfault at 4 ip 00000000567c7c98
>>>>>> sp 00000000ff937ae0 error 4 in perf[56630000+1b2000]
>>>>>> [ 208.724778] Code: fc ff ff e8 aa 9b 01 00 8d b4 26 00 00 00 00 8d
>>>>>> 76 00 55 89 e5 83 ec 18 65 8b 0d 14 00 00 00 89
>>>>>> 4d f4 31 c9 8b 45 08 8b9
>>>>>> Segmentation fault
>>>>> I'm not being able to repro this here, what is the tree you are using?
>>>> I believe that's the same bug that Jann Horn reported recently for perf trace.
>>>> I thought the patch for that went in.
>>> Ok, Zhe, that patch is at the end of this message, and it is in:
>>>
>>> [acme@five perf]$ git tag --contains cb71f7d43ece3d5a4f400f510c61b2ec7c9ce9a1 | grep ^v
>>> v5.6-rc1
>>> v5.6-rc2
>>> v5.6-rc3
>>> v5.6-rc4
>>> [acme@five perf]$
>>>
>>> Can you try with that?
>> Thanks, that does fix the issue I met.
>>
>> BTW, my change in perf_cpu_map__cpu can be used as a preventive check
>> and the "1" in perf_cpu_map__cpu should be "0", and assigning a NULL in
> I agree, can't see why we had 1 in here.. must be connected to the dummy
> map.. could you please double check with all the perf_cpu_map__nr usages
> that the 0 will work as expected?
I just checked the callers of perf_cpu_map__nr. They really depend on it
returning 1 as the only one cpu at least. And the same trick is played in
perf_thread_map__nr. So perf_cpu_map__nr should remain unchanged.
I'll send v2 for the rest of the hunks.
Thanks,
Zhe
>
>> perf_evlist__exit makes the clearing complete. So are they worth a new patch?
> the rest of the hunks looks good as preventive checks
>
> thanks,
> jirka
>