linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/perf: Fix addr location init during arch_skip_callchain_idx function
@ 2023-07-24 16:58 Athira Rajeev
  2023-07-24 21:03 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Athira Rajeev @ 2023-07-24 16:58 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: atrajeev, kjain, linux-perf-users, maddy, Aneesh Kumar K . V,
	disgoel, linuxppc-dev

perf record with callchain recording fails as below
in powerpc:

    ./perf record -a -gR sleep 10
    ./perf report
    perf: Segmentation fault

gdb trace points to thread__find_map

    0  0x00000000101df314 in atomic_cmpxchg (newval=1818846826, oldval=1818846827, v=0x1001a8f3) at /home/athira/linux/tools/include/asm-generic/atomic-gcc.h:70
    1  refcount_sub_and_test (i=1, r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:135
    2  refcount_dec_and_test (r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:148
    3  map__put (map=0x1001a8b3) at util/map.c:311
    4  0x000000001016842c in __map__zput (map=0x7fffffffa368) at util/map.h:190
    5  thread__find_map (thread=0x105b92f0, cpumode=<optimized out>, addr=13835058055283572736, al=al@entry=0x7fffffffa358) at util/event.c:582
    6  0x000000001016882c in thread__find_symbol (thread=<optimized out>, cpumode=<optimized out>, addr=<optimized out>, al=0x7fffffffa358) at util/event.c:656
    7  0x00000000102e12b4 in arch_skip_callchain_idx (thread=<optimized out>, chain=<optimized out>) at arch/powerpc/util/skip-callchain-idx.c:255
    8  0x00000000101d3bf4 in thread__resolve_callchain_sample (thread=0x105b92f0, cursor=0x1053d160, evsel=<optimized out>, sample=0x7fffffffa908, parent=0x7fffffffa778, root_al=0x7fffffffa710,
        max_stack=<optimized out>) at util/machine.c:2940
    9  0x00000000101cd210 in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=<optimized out>, evsel=<optimized out>, al=<optimized out>, max_stack=<optimized out>)
        at util/callchain.c:1112
    10 0x000000001022a9d8 in hist_entry_iter__add (iter=0x7fffffffa750, al=0x7fffffffa710, max_stack_depth=<optimized out>, arg=0x7fffffffbbd0) at util/hist.c:1232
    11 0x0000000010056d98 in process_sample_event (tool=0x7fffffffbbd0, event=0x7ffff6223c38, sample=0x7fffffffa908, evsel=<optimized out>, machine=0x10524ef8) at builtin-report.c:332

Here arch_skip_callchain_idx calls thread__find_symbol and which
invokes thread__find_map with uninitialised "addr_location".
Snippet:

thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);

Recent change with commit 0dd5041c9a0ea ("perf addr_location:
Add init/exit/copy functions"), introduced "maps__zput" in the
function thread__find_map. This could result in segfault while
accessing uninitialised map from "struct addr_location". Fix this
by adding addr_location__init and addr_location__exit in
arch_skip_callchain_idx.

Fixes: 0dd5041c9a0ea ("perf addr_location: Add init/exit/copy functions")
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/skip-callchain-idx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/skip-callchain-idx.c b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
index b7223feec770..5f3edb3004d8 100644
--- a/tools/perf/arch/powerpc/util/skip-callchain-idx.c
+++ b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
@@ -250,6 +250,7 @@ int arch_skip_callchain_idx(struct thread *thread, struct ip_callchain *chain)
 	if (!chain || chain->nr < 3)
 		return skip_slot;
 
+	addr_location__init(&al);
 	ip = chain->ips[1];
 
 	thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);
@@ -259,6 +260,7 @@ int arch_skip_callchain_idx(struct thread *thread, struct ip_callchain *chain)
 
 	if (!dso) {
 		pr_debug("%" PRIx64 " dso is NULL\n", ip);
+		addr_location__exit(&al);
 		return skip_slot;
 	}
 
@@ -279,5 +281,7 @@ int arch_skip_callchain_idx(struct thread *thread, struct ip_callchain *chain)
 		 */
 		skip_slot = 3;
 	}
+
+	addr_location__exit(&al);
 	return skip_slot;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/perf: Fix addr location init during arch_skip_callchain_idx function
  2023-07-24 16:58 [PATCH] tools/perf: Fix addr location init during arch_skip_callchain_idx function Athira Rajeev
@ 2023-07-24 21:03 ` Arnaldo Carvalho de Melo
  2023-07-25 11:09   ` Athira Rajeev
  0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-07-24 21:03 UTC (permalink / raw)
  To: Athira Rajeev, Adrian Hunter
  Cc: irogers, maddy, kjain, linux-perf-users, jolsa,
	Aneesh Kumar K . V, namhyung, disgoel, linuxppc-dev

Em Mon, Jul 24, 2023 at 10:28:15PM +0530, Athira Rajeev escreveu:
> perf record with callchain recording fails as below
> in powerpc:
> 
>     ./perf record -a -gR sleep 10
>     ./perf report
>     perf: Segmentation fault
> 
> gdb trace points to thread__find_map
> 
>     0  0x00000000101df314 in atomic_cmpxchg (newval=1818846826, oldval=1818846827, v=0x1001a8f3) at /home/athira/linux/tools/include/asm-generic/atomic-gcc.h:70
>     1  refcount_sub_and_test (i=1, r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:135
>     2  refcount_dec_and_test (r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:148
>     3  map__put (map=0x1001a8b3) at util/map.c:311
>     4  0x000000001016842c in __map__zput (map=0x7fffffffa368) at util/map.h:190
>     5  thread__find_map (thread=0x105b92f0, cpumode=<optimized out>, addr=13835058055283572736, al=al@entry=0x7fffffffa358) at util/event.c:582
>     6  0x000000001016882c in thread__find_symbol (thread=<optimized out>, cpumode=<optimized out>, addr=<optimized out>, al=0x7fffffffa358) at util/event.c:656
>     7  0x00000000102e12b4 in arch_skip_callchain_idx (thread=<optimized out>, chain=<optimized out>) at arch/powerpc/util/skip-callchain-idx.c:255
>     8  0x00000000101d3bf4 in thread__resolve_callchain_sample (thread=0x105b92f0, cursor=0x1053d160, evsel=<optimized out>, sample=0x7fffffffa908, parent=0x7fffffffa778, root_al=0x7fffffffa710,
>         max_stack=<optimized out>) at util/machine.c:2940
>     9  0x00000000101cd210 in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=<optimized out>, evsel=<optimized out>, al=<optimized out>, max_stack=<optimized out>)
>         at util/callchain.c:1112
>     10 0x000000001022a9d8 in hist_entry_iter__add (iter=0x7fffffffa750, al=0x7fffffffa710, max_stack_depth=<optimized out>, arg=0x7fffffffbbd0) at util/hist.c:1232
>     11 0x0000000010056d98 in process_sample_event (tool=0x7fffffffbbd0, event=0x7ffff6223c38, sample=0x7fffffffa908, evsel=<optimized out>, machine=0x10524ef8) at builtin-report.c:332
> 
> Here arch_skip_callchain_idx calls thread__find_symbol and which
> invokes thread__find_map with uninitialised "addr_location".
> Snippet:
> 
> thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);
> 
> Recent change with commit 0dd5041c9a0ea ("perf addr_location:
> Add init/exit/copy functions"), introduced "maps__zput" in the
> function thread__find_map. This could result in segfault while
> accessing uninitialised map from "struct addr_location". Fix this
> by adding addr_location__init and addr_location__exit in
> arch_skip_callchain_idx.

Thanks, applied.
 
> Fixes: 0dd5041c9a0ea ("perf addr_location: Add init/exit/copy functions")

> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

I'll also do a audit of all calls to thread__find_map() and its callers
to check for other such cases :-\

For instance, this one seem buggy as well, Adrian?

diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c
index 46f74b2344dbb34c..798a53d7e6c9dfc5 100644
--- a/tools/perf/util/dlfilter.c
+++ b/tools/perf/util/dlfilter.c
@@ -166,6 +166,7 @@ static __s32 dlfilter__resolve_address(void *ctx, __u64 address, struct perf_dlf
 	if (!thread)
 		return -1;
 
+	addr_location__init(&al);
 	thread__find_symbol_fb(thread, d->sample->cpumode, address, &al);
 
 	al_to_d_al(&al, &d_al);



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/perf: Fix addr location init during arch_skip_callchain_idx function
  2023-07-24 21:03 ` Arnaldo Carvalho de Melo
@ 2023-07-25 11:09   ` Athira Rajeev
  0 siblings, 0 replies; 3+ messages in thread
From: Athira Rajeev @ 2023-07-25 11:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Madhavan Srinivasan, Kajol Jain, Adrian Hunter,
	linux-perf-users, Jiri Olsa, Aneesh Kumar K . V, Namhyung Kim,
	Disha Goel, linuxppc-dev



> On 25-Jul-2023, at 2:33 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> Em Mon, Jul 24, 2023 at 10:28:15PM +0530, Athira Rajeev escreveu:
>> perf record with callchain recording fails as below
>> in powerpc:
>> 
>>   ./perf record -a -gR sleep 10
>>   ./perf report
>>   perf: Segmentation fault
>> 
>> gdb trace points to thread__find_map
>> 
>>   0  0x00000000101df314 in atomic_cmpxchg (newval=1818846826, oldval=1818846827, v=0x1001a8f3) at /home/athira/linux/tools/include/asm-generic/atomic-gcc.h:70
>>   1  refcount_sub_and_test (i=1, r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:135
>>   2  refcount_dec_and_test (r=0x1001a8f3) at /home/athira/linux/tools/include/linux/refcount.h:148
>>   3  map__put (map=0x1001a8b3) at util/map.c:311
>>   4  0x000000001016842c in __map__zput (map=0x7fffffffa368) at util/map.h:190
>>   5  thread__find_map (thread=0x105b92f0, cpumode=<optimized out>, addr=13835058055283572736, al=al@entry=0x7fffffffa358) at util/event.c:582
>>   6  0x000000001016882c in thread__find_symbol (thread=<optimized out>, cpumode=<optimized out>, addr=<optimized out>, al=0x7fffffffa358) at util/event.c:656
>>   7  0x00000000102e12b4 in arch_skip_callchain_idx (thread=<optimized out>, chain=<optimized out>) at arch/powerpc/util/skip-callchain-idx.c:255
>>   8  0x00000000101d3bf4 in thread__resolve_callchain_sample (thread=0x105b92f0, cursor=0x1053d160, evsel=<optimized out>, sample=0x7fffffffa908, parent=0x7fffffffa778, root_al=0x7fffffffa710,
>>       max_stack=<optimized out>) at util/machine.c:2940
>>   9  0x00000000101cd210 in sample__resolve_callchain (sample=<optimized out>, cursor=<optimized out>, parent=<optimized out>, evsel=<optimized out>, al=<optimized out>, max_stack=<optimized out>)
>>       at util/callchain.c:1112
>>   10 0x000000001022a9d8 in hist_entry_iter__add (iter=0x7fffffffa750, al=0x7fffffffa710, max_stack_depth=<optimized out>, arg=0x7fffffffbbd0) at util/hist.c:1232
>>   11 0x0000000010056d98 in process_sample_event (tool=0x7fffffffbbd0, event=0x7ffff6223c38, sample=0x7fffffffa908, evsel=<optimized out>, machine=0x10524ef8) at builtin-report.c:332
>> 
>> Here arch_skip_callchain_idx calls thread__find_symbol and which
>> invokes thread__find_map with uninitialised "addr_location".
>> Snippet:
>> 
>> thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);
>> 
>> Recent change with commit 0dd5041c9a0ea ("perf addr_location:
>> Add init/exit/copy functions"), introduced "maps__zput" in the
>> function thread__find_map. This could result in segfault while
>> accessing uninitialised map from "struct addr_location". Fix this
>> by adding addr_location__init and addr_location__exit in
>> arch_skip_callchain_idx.
> 
> Thanks, applied.

Hi Arnaldo,

Thanks for picking the fix 

Athira
> 
>> Fixes: 0dd5041c9a0ea ("perf addr_location: Add init/exit/copy functions")
> 
>> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> 
> I'll also do a audit of all calls to thread__find_map() and its callers
> to check for other such cases :-\
> 
> For instance, this one seem buggy as well, Adrian?
> 
> diff --git a/tools/perf/util/dlfilter.c b/tools/perf/util/dlfilter.c
> index 46f74b2344dbb34c..798a53d7e6c9dfc5 100644
> --- a/tools/perf/util/dlfilter.c
> +++ b/tools/perf/util/dlfilter.c
> @@ -166,6 +166,7 @@ static __s32 dlfilter__resolve_address(void *ctx, __u64 address, struct perf_dlf
> if (!thread)
> return -1;
> 
> + addr_location__init(&al);
> thread__find_symbol_fb(thread, d->sample->cpumode, address, &al);
> 
> al_to_d_al(&al, &d_al);



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-07-25 11:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-24 16:58 [PATCH] tools/perf: Fix addr location init during arch_skip_callchain_idx function Athira Rajeev
2023-07-24 21:03 ` Arnaldo Carvalho de Melo
2023-07-25 11:09   ` Athira Rajeev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).