perf/urgent fixes
Subsystems (2)
Arnaldo Carvalho de Melo
maintainerSee http://www.infradead.org/rpr.html
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit 28fb8a5b6e233fc384fb27f9f91f811b40ba9cf8:
Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2016-01-30 09:15:49 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
for you to fetch changes up to d3a37ea00a9fbc0f4fa91647c9ae66c038ba65e3:
perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info() (2016-02-01 12:03:35 -0300)
----------------------------------------------------------------
perf/urgent fixes:
- tracepoint_error() can receive e=NULL, robustify it, fixes a problem noticed with a very specific combination: Machine with Intel PT (e.g. Broadwell), kernel with no perf_event_attr.context_switch feature (e.g. 4.2) and unreadable tracefs (for instance !root users), making the fallback from perf_event_attr.context_switch to the sched:sched_switch tracepoint to fail reading its info from tracefs, fix it. (Adrian Hunter)
- Fix fault in error path of intel_pt_process_auxtrace_info(), obeying the existing 'struct thread' lifetime cycle checking expectations, noticed for instance, when processing perf.data files with Intel PT data using 'perf script' (Wang Nan)
----------------------------------------------------------------
Adrian Hunter (1):
perf tools: tracepoint_error() can receive e=NULL, robustify it
Wang Nan (1):
perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
tools/perf/util/intel-pt.c | 4 ++++ tools/perf/util/parse-events.c | 3 +++ 2 files changed, 7 insertions(+)
Arnaldo Carvalho de Melo
maintainerPATCH 1/2 perf tools: tracepoint_error() can receive e=NULL, robustify it
From: Adrian Hunter <[email protected]>
Fixes segmentation fault using, for instance:
(gdb) run record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls
Starting program: /home/acme/bin/perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls Missing separate debuginfos, use: dnf debuginfo-install glibc-2.22-7.fc23.x86_64 [Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Program received signal SIGSEGV, Segmentation fault.
0 x00000000004b9ea5 in tracepoint_error (e=0x0, err=13, sys=0x19b1370 "sched", name=0x19a5d00 "sched_switch") at util/parse-events.c:410 (gdb) bt
#0 0x00000000004b9ea5 in tracepoint_error (e=0x0, err=13, sys=0x19b1370 "sched", name=0x19a5d00 "sched_switch") at util/parse-events.c:410 #1 0x00000000004b9fc5 in add_tracepoint (list=0x19a5d20, idx=0x7fffffffb8c0, sys_name=0x19b1370 "sched", evt_name=0x19a5d00 "sched_switch", err=0x0, head_config=0x0)
at util/parse-events.c:433
#2 0x00000000004ba334 in add_tracepoint_event (list=0x19a5d20, idx=0x7fffffffb8c0, sys_name=0x19b1370 "sched", evt_name=0x19a5d00 "sched_switch", err=0x0, head_config=0x0)
at util/parse-events.c:498
#3 0x00000000004bb699 in parse_events_add_tracepoint (list=0x19a5d20, idx=0x7fffffffb8c0, sys=0x19b1370 "sched", event=0x19a5d00 "sched_switch", err=0x0, head_config=0x0)
at util/parse-events.c:936
#4 0x00000000004f6eda in parse_events_parse (_data=0x7fffffffb8b0, scanner=0x19a49d0) at util/parse-events.y:391
#5 0x00000000004bc8e5 in parse_events__scanner (str=0x663ff2 "sched:sched_switch", data=0x7fffffffb8b0, start_token=258) at util/parse-events.c:1361
#6 0x00000000004bca57 in parse_events (evlist=0x19a5220, str=0x663ff2 "sched:sched_switch", err=0x0) at util/parse-events.c:1401
#7 0x0000000000518d5f in perf_evlist__can_select_event (evlist=0x19a3b90, str=0x663ff2 "sched:sched_switch") at util/record.c:253
#8 0x0000000000553c42 in intel_pt_track_switches (evlist=0x19a3b90) at arch/x86/util/intel-pt.c:364
#9 0x00000000005549d1 in intel_pt_recording_options (itr=0x19a2c40, evlist=0x19a3b90, opts=0x8edf68 <record+232>) at arch/x86/util/intel-pt.c:664
#10 0x000000000051e076 in auxtrace_record__options (itr=0x19a2c40, evlist=0x19a3b90, opts=0x8edf68 <record+232>) at util/auxtrace.c:539
#11 0x0000000000433368 in cmd_record (argc=1, argv=0x7fffffffde60, prefix=0x0) at builtin-record.c:1264
#12 0x000000000049bec2 in run_builtin (p=0x8fa2a8 <commands+168>, argc=5, argv=0x7fffffffde60) at perf.c:390
#13 0x000000000049c12a in handle_internal_command (argc=5, argv=0x7fffffffde60) at perf.c:451
#14 0x000000000049c278 in run_argv (argcp=0x7fffffffdcbc, argv=0x7fffffffdcb0) at perf.c:495
#15 0x000000000049c60a in main (argc=5, argv=0x7fffffffde60) at perf.c:618(gdb)
Intel PT attempts to find the sched:sched_switch tracepoint but that seg faults if tracefs is not readable, because the error reporting structure is null, as errors are not reported when automatically adding tracepoints. Fix by checking before using.
Committer note:
This doesn't take place in a kernel that supports perf_event_attr.context_switch, that is the default way that will be used for tracking context switches, only in older kernels, like 4.2, in a machine with Intel PT (e.g. Broadwell) for non-priviledged users.
Further info from a similar patch by Wang:
The error is in tracepoint_error: it assumes the 'e' parameter is valid.
However, there are many situation a parse_event() can be called without parse_events_error. See result of
$ grep 'parse_events(.*NULL)' ./tools/perf/ -r'
--- tools/perf/util/parse-events.c | 3 +++ 1 file changed, 3 insertions(+)
tools/perf/util/parse-events.c +3 -0
@@ -399,6 +399,9 @@ static void tracepoint_error(struct parse_events_error *e, int err, {char help[BUFSIZ ]; +if (!e) +return ; +/* * We get error directly from syscall errno ( > 0), * or from encoded pointer's error ( < 0).
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 2/2 perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
In error processing path of intel_pt_process_auxtrace_info() it calls thread__zput() to clean and free pt->unknown_thread which is created by thread__new(). However, when error raise, a segfault happen:
# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #
The problem is: there's a union in 'struct thread' combines a list_head and a rb_node. The standard life cycle of a thread is: init rb_node during creating, inserted into machine->threads rbtree uses rb_node, move to machine->dead_threads using list_head, clean by thread__put:list_del_init(&thread->node).
In the above command, it clean a thread before adding it into list, causes the above segfault.
This patch gives a fake list_head and link the thread into it before calling thread__zput(), get rid of the segfault.
After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #
--- tools/perf/util/intel-pt.c | 4 ++++ 1 file changed, 4 insertions(+)
tools/perf/util/intel-pt.c +4 -0
@@ -2013,6 +2013,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,struct auxtrace_info_event *auxtrace_info = &event->auxtrace_info;size_t min_sz =sizeof (u64) *INTEL_PT_PER_CPU_MMAPS ;struct intel_pt *pt; +struct list_head dead_thread;int err; if (auxtrace_info->header.size < sizeof(struct auxtrace_info_event) +
@@ -2153,6 +2154,9 @@ int intel_pt_process_auxtrace_info(union perf_event *event,return 0 ; err_delete_thread: +RB_CLEAR_NODE (&pt->unknown_thread->rb_node); +INIT_LIST_HEAD (&dead_thread); +list_add (&pt->unknown_thread->node, &dead_thread);thread__zput (pt->unknown_thread); err_free_queues:intel_pt_log_disable ();
Signature · 1 line
2.5.0
Adrian Hunter
reviewerRe: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
This patch does not fix the problem because the thread__zput() will still segfault later if the error path is not taken.
Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
http://marc.info/?l=linux-kernel&m=145431692623940
However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
tools/perf/util/intel-pt.c +1 -0
@@ -2068,6 +2068,7 @@ int intel_pt_process_auxtrace_info(union perf_event err = -ENOMEM ; goto err_free_queues; } +INIT_LIST_HEAD (&pt->unknown_thread->node); err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;
Quoted 71 lines · Arnaldo Carvalho de Melo
From: Wang Nan <[email protected]>
In error processing path of
intel_pt_process_auxtrace_info()it callsthread__zput()to clean and freept->unknown_threadwhich is created bythread__new(). However, when error raise, a segfault happen:# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #The problem is: there's a union in 'struct thread' combines a
list_headand arb_node. The standard life cycle of a thread is: initrb_nodeduring creating, inserted intomachine->threadsrbtree usesrb_node, move tomachine->dead_threadsusinglist_head, clean bythread__put:list_del_init(&thread->node).In the above command, it clean a thread before adding it into list, causes the above segfault.
This patch gives a fake
list_headand link the thread into it before callingthread__zput(), get rid of the segfault.After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #Reported-by Tong Zhang <[email protected]>Signed-off-by Wang Nan <[email protected]>Cc Adrian Hunter <[email protected]>Cc Josh Poimboeuf <[email protected]>Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/intel-pt.c | 4 ++++ 1 file changed, 4 insertions(+)tools/perf/util/intel-pt.c +4
-0@@ -2013,6 +2013,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,struct auxtrace_info_event *auxtrace_info = &event->auxtrace_info;size_t min_sz =sizeof (u64) *INTEL_PT_PER_CPU_MMAPS ;struct intel_pt *pt; +struct list_head dead_thread;int err; if (auxtrace_info->header.size < sizeof(struct auxtrace_info_event) +@@ -2153,6 +2154,9 @@ int intel_pt_process_auxtrace_info(union perf_event *event,return 0 ; err_delete_thread: +RB_CLEAR_NODE (&pt->unknown_thread->rb_node); +INIT_LIST_HEAD (&dead_thread); +list_add (&pt->unknown_thread->node, &dead_thread);thread__zput (pt->unknown_thread); err_free_queues:intel_pt_log_disable ();
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
Em Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu:
Quoted 11 lines
This patch does not fix the problem because the
thread__zput()will still segfault later if the error path is not taken.Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
http://marc.info/?l=linux-kernel&m=145431692623940However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
Right, so it keeps the existing thread lifetime assumptions both in the error path, as Wang did, and in the normal pt->unknown_thread end of life at intel_pt_free(), I'm replacing the patch...
- Arnaldo
Quoted 88 lines
tools/perf/util/intel-pt.c +1
-0@@ -2068,6 +2068,7 @@ int intel_pt_process_auxtrace_info(union perf_event err = -ENOMEM ; goto err_free_queues; } +INIT_LIST_HEAD (&pt->unknown_thread->node); err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;Quoted 71 lines · Arnaldo Carvalho de Melo
From: Wang Nan <[email protected]>
In error processing path of
intel_pt_process_auxtrace_info()it callsthread__zput()to clean and freept->unknown_threadwhich is created bythread__new(). However, when error raise, a segfault happen:# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #The problem is: there's a union in 'struct thread' combines a
list_headand arb_node. The standard life cycle of a thread is: initrb_nodeduring creating, inserted intomachine->threadsrbtree usesrb_node, move tomachine->dead_threadsusinglist_head, clean bythread__put:list_del_init(&thread->node).In the above command, it clean a thread before adding it into list, causes the above segfault.
This patch gives a fake
list_headand link the thread into it before callingthread__zput(), get rid of the segfault.After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #Reported-by Tong Zhang <[email protected]>Signed-off-by Wang Nan <[email protected]>Cc Adrian Hunter <[email protected]>Cc Josh Poimboeuf <[email protected]>Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/intel-pt.c | 4 ++++ 1 file changed, 4 insertions(+)tools/perf/util/intel-pt.c +4
-0@@ -2013,6 +2013,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,struct auxtrace_info_event *auxtrace_info = &event->auxtrace_info;size_t min_sz =sizeof (u64) *INTEL_PT_PER_CPU_MMAPS ;struct intel_pt *pt; +struct list_head dead_thread;int err; if (auxtrace_info->header.size < sizeof(struct auxtrace_info_event) +@@ -2153,6 +2154,9 @@ int intel_pt_process_auxtrace_info(union perf_event *event,return 0 ; err_delete_thread: +RB_CLEAR_NODE (&pt->unknown_thread->rb_node); +INIT_LIST_HEAD (&dead_thread); +list_add (&pt->unknown_thread->node, &dead_thread);thread__zput (pt->unknown_thread); err_free_queues:intel_pt_log_disable ();
Re: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
In reply to Adrian HunterEm Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu:
Quoted 11 lines
This patch does not fix the problem because the
thread__zput()will still segfault later if the error path is not taken.Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
http://marc.info/?l=linux-kernel&m=145431692623940However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
So, can I go with the following patch+description+authorship?
Quoted 1 line
From
3a4acda1ecbd290973de08250d7dcdfaf5b2fe0fMon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Mon, 1 Feb 2016 03:21:04 +0000
Subject: [PATCH 1/1] perf tools: Fix thread lifetime related segfaut in intel_pt
intel_pt_process_auxtrace_info() creates a pt->unknown_thread thread that eventually needs to be freed by the last thread__put() on it, when its refcount hits zero, which may happen in intel_pt_process_auxtrace_info() error handling path and triggers the following segfault, which would happen as well at intel_pt_free, when tools using this intel_pt codebase frees up resources:
# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #
The problem is: there's a union in 'struct thread' combines a list_head and a rb_node. The standard life cycle of a thread is: init rb_node in the constructor, insert it into machine->threads rbtree using rb_node, move it to machine->dead_threads using list_head, clean in the lastthread__put: list_del_init(&thread->node).
In the above command, it clean a thread before adding it into list, causes the above segfault.
Since pt->unknown_thread will never live in an rbtree, initialize its list node so that when list_del_init() is done on it we don't segfault.
After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #
--- tools/perf/util/intel-pt.c | 9 +++++++++ 1 file changed, 9 insertions(+)
tools/perf/util/intel-pt.c +9 -0
@@ -2068,6 +2068,15 @@ int intel_pt_process_auxtrace_info(union perf_event *event, err = -ENOMEM ; goto err_free_queues; } + +/* +* Since this thread will not be kept in any rbtree not in a +* list, initialize its list node so that at thread__put() the +* current thread lifetime assuption is kept and we don't segfault +* at list_del_init(). +*/ +INIT_LIST_HEAD (&pt->unknown_thread->node); + err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
Em Tue, Feb 02, 2016 at 12:52:25PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 7 lines
Em Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu:
Quoted 6 lines
This patch does not fix the problem because the
thread__zput()will still segfault later if the error path is not taken.Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
Quoted 7 lines
Quoted 5 lines
http://marc.info/?l=linux-kernel&m=145431692623940However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
So, can I go with the following patch+description+authorship?
BTW, I just noticed it while testing some unrelated patch (Jiri's hpp hists stuff) that with a perf.data file with intel pt data, I get this at the end of a TUI session, i.e. the segfault at tool exit:
Available samples
0 intel_pt/tsc=1,noretcomp=1/u ◆ 29 sched:sched_switch ▒ 0 dummy:u ▒ 6 instructions:u ▒ 0 transactions ▒
Program received signal SIGSEGV, Segmentation fault. ▒
0x00000000004ec5c5 in __write_once_size (size=8, res=0x7fffffffc2e0, p=0x0) at /home/acme/git/linux/tools/include/linux/compiler.h:82 ▒
82 case 8: *(volatile __u64_alias_t *) p = *(__u64_alias_t *) res; break; ▒
Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.4.5-1.fc23.x86_64 bzip2-libs-1.0.6-19.fc23.x86_64 elfutils-libelf-0.165-2.fc23.x86_64 elfutils-libs-0.165-2.fc23.x86_64 libunwind-1.1-10.fc23.x86_64 nss-softokn-freebl-3.21.0-1.1.fc23.x86_64 numactl-libs-2.0.10-3.fc23.x86_64 perl-libs-5.22.1-350.fc23.x86_64 python-libs-2.7.10-8.fc23.x86_64 slang-2.3.0-4.fc23.x86_64 xz-libs-5.2.1-3.fc23.x86_64 zlib-1.2.8-9.fc23.x86_64 ▒
(gdb) bt ▒ #0 0x00000000004ec5c5 in __write_once_size (size=8, res=0x7fffffffc2e0, p=0x0) at /home/acme/git/linux/tools/include/linux/compiler.h:82 ▒ #1 __list_del (prev=0x0, next=0x1a2e970) at /home/acme/git/linux/tools/include/linux/list.h:89 ▒ #2 0x00000000004ec631 in __list_del_entry (entry=0x1a2e970) at /home/acme/git/linux/tools/include/linux/list.h:101 ▒ #3 0x00000000004ec6d2 in list_del_init (entry=0x1a2e970) at /home/acme/git/linux/tools/include/linux/list.h:144 ▒ #4 0x00000000004ecd2a in thread__put (thread=0x1a2e970) at util/thread.c:104 ▒ #5 0x000000000052de51 in intel_pt_free (session=0x19a3b90) at util/intel-pt.c:1747 ▒ #6 0x00000000004e4e3d in auxtrace__free (session=0x19a3b90) at util/auxtrace.h:513 ▒ #7 0x00000000004e5603 in perf_session__delete (session=0x19a3b90) at util/session.c:181 ▒ #8 0x00000000004363f3 in cmd_report (argc=0, argv=0x7fffffffdeb0, prefix=0x0) at builtin-report.c:984 ▒ #9 0x000000000049bea3 in run_builtin (p=0x8fa2c0 <commands+192>, argc=1, argv=0x7fffffffdeb0) at perf.c:390 ▒ #10 0x000000000049c10b in handle_internal_command (argc=1, argv=0x7fffffffdeb0) at perf.c:451 ▒ #11 0x000000000049c259 in run_argv (argcp=0x7fffffffdd0c, argv=0x7fffffffdd00) at perf.c:495 ▒ #12 0x000000000049c5eb in main (argc=1, argv=0x7fffffffdeb0) at perf.c:618 ▒
(gdb)
Quoted 77 lines
From
3a4acda1ecbd290973de08250d7dcdfaf5b2fe0fMon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Mon, 1 Feb 2016 03:21:04 +0000
Subject: [PATCH 1/1] perf tools: Fix thread lifetime related segfaut inintel_pt
intel_pt_process_auxtrace_info()creates apt->unknown_threadthread that eventually needs to be freed by the lastthread__put()on it, when its refcount hits zero, which may happen inintel_pt_process_auxtrace_info()error handling path and triggers the following segfault, which would happen as well atintel_pt_free, when tools using thisintel_ptcodebase frees up resources:# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #The problem is: there's a union in 'struct thread' combines a
list_headand arb_node. The standard life cycle of a thread is: initrb_nodein the constructor, insert it intomachine->threadsrbtree usingrb_node, move it tomachine->dead_threadsusinglist_head, clean in the lastthread__put:list_del_init(&thread->node).In the above command, it clean a thread before adding it into list, causes the above segfault.
Since
pt->unknown_threadwill never live in an rbtree, initialize its list node so that whenlist_del_init()is done on it we don't segfault.After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #Reported-by Tong Zhang <[email protected]>Reported-by Wang Nan <[email protected]>Signed-off-by Adrian Hunter <[email protected]>Cc Josh Poimboeuf <[email protected]>Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/intel-pt.c | 9 +++++++++ 1 file changed, 9 insertions(+)tools/perf/util/intel-pt.c +9
-0@@ -2068,6 +2068,15 @@ int intel_pt_process_auxtrace_info(union perf_event *event, err = -ENOMEM ; goto err_free_queues; } + +/* +* Since this thread will not be kept in any rbtree not in a +* list, initialize its list node so that at thread__put() the +* current thread lifetime assuption is kept and we don't segfault +* at list_del_init(). +*/ +INIT_LIST_HEAD (&pt->unknown_thread->node); + err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;Signature · 1 line
2.5.0
Re: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
In reply to Arnaldo Carvalho de MeloQuoted 7 lines · Arnaldo Carvalho de Melo
Em Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu:
Quoted 6 lines
This patch does not fix the problem because the
thread__zput()will still segfault later if the error path is not taken.Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
Quoted 6 lines
Quoted 5 lines
http://marc.info/?l=linux-kernel&m=145431692623940However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
So, can I go with the following patch+description+authorship?
I didn't really understand the lifecycle of unknown_thread, and though it would go to an rbtree in normal path. I think the patch you posted is good.
Thank you.
Quoted 74 lines
From
3a4acda1ecbd290973de08250d7dcdfaf5b2fe0fMon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Mon, 1 Feb 2016 03:21:04 +0000
Subject: [PATCH 1/1] perf tools: Fix thread lifetime related segfaut inintel_pt
intel_pt_process_auxtrace_info()creates apt->unknown_threadthread that eventually needs to be freed by the lastthread__put()on it, when its refcount hits zero, which may happen inintel_pt_process_auxtrace_info()error handling path and triggers the following segfault, which would happen as well atintel_pt_free, when tools using thisintel_ptcodebase frees up resources:# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #The problem is: there's a union in 'struct thread' combines a
list_headand arb_node. The standard life cycle of a thread is: initrb_nodein the constructor, insert it intomachine->threadsrbtree usingrb_node, move it tomachine->dead_threadsusinglist_head, clean in the lastthread__put:list_del_init(&thread->node).In the above command, it clean a thread before adding it into list, causes the above segfault.
Since
pt->unknown_threadwill never live in an rbtree, initialize its list node so that whenlist_del_init()is done on it we don't segfault.After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #Reported-by Tong Zhang <[email protected]>Reported-by Wang Nan <[email protected]>Signed-off-by Adrian Hunter <[email protected]>Cc Josh Poimboeuf <[email protected]>Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/intel-pt.c | 9 +++++++++ 1 file changed, 9 insertions(+)tools/perf/util/intel-pt.c +9
-0@@ -2068,6 +2068,15 @@ int intel_pt_process_auxtrace_info(union perf_event *event, err = -ENOMEM ; goto err_free_queues; } + +/* +* Since this thread will not be kept in any rbtree not in a +* list, initialize its list node so that at thread__put() the +* current thread lifetime assuption is kept and we don't segfault +* at list_del_init(). +*/ +INIT_LIST_HEAD (&pt->unknown_thread->node); + err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;
Adrian Hunter
reviewerRe: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info()
In reply to Arnaldo Carvalho de MeloQuoted 7 lines · Arnaldo Carvalho de Melo
Em Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu:
Quoted 6 lines
This patch does not fix the problem because the
thread__zput()will still segfault later if the error path is not taken.Sorry, I didn't look closely at this patch because I was not expecting it to be taken because of the fix I had already sent:
Quoted 7 lines
Quoted 5 lines
http://marc.info/?l=linux-kernel&m=145431692623940However if you want to keep the struct thread rbtree / list union, the simple fix would be to reinstate the list initialization in this particular case i.e.:
So, can I go with the following patch+description+authorship?
Fine by me. Thank you!
Quoted 76 lines
From
3a4acda1ecbd290973de08250d7dcdfaf5b2fe0fMon Sep 17 00:00:00 2001
From: Adrian Hunter <[email protected]>
Date: Mon, 1 Feb 2016 03:21:04 +0000
Subject: [PATCH 1/1] perf tools: Fix thread lifetime related segfaut inintel_pt
intel_pt_process_auxtrace_info()creates apt->unknown_threadthread that eventually needs to be freed by the lastthread__put()on it, when its refcount hits zero, which may happen inintel_pt_process_auxtrace_info()error handling path and triggers the following segfault, which would happen as well atintel_pt_free, when tools using thisintel_ptcodebase frees up resources:# perf record -I -e intel_pt/tsc=1,noretcomp=1/u /bin/ls 0 a anaconda-ks.cfg bin perf.data perf.data.old perf-f23-bringup.todo [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.217 MB perf.data ] # # perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type Segmentation fault (core dumped) #The problem is: there's a union in 'struct thread' combines a
list_headand arb_node. The standard life cycle of a thread is: initrb_nodein the constructor, insert it intomachine->threadsrbtree usingrb_node, move it tomachine->dead_threadsusinglist_head, clean in the lastthread__put:list_del_init(&thread->node).In the above command, it clean a thread before adding it into list, causes the above segfault.
Since
pt->unknown_threadwill never live in an rbtree, initialize its list node so that whenlist_del_init()is done on it we don't segfault.After this patch:
# perf script -F event,comm,pid,tid,time,addr,ip,sym,dso,iregs Samples for 'instructions:u' event do not have IREGS attribute set. Cannot print 'iregs' field. intel_pt_synth_events: failed to synthesize 'instructions' event type 0x248 [0x88]: failed to process type: 70 #Reported-by Tong Zhang <[email protected]>Reported-by Wang Nan <[email protected]>Signed-off-by Adrian Hunter <[email protected]>Cc Josh Poimboeuf <[email protected]>Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/intel-pt.c | 9 +++++++++ 1 file changed, 9 insertions(+)tools/perf/util/intel-pt.c +9
-0@@ -2068,6 +2068,15 @@ int intel_pt_process_auxtrace_info(union perf_event *event, err = -ENOMEM ; goto err_free_queues; } + +/* +* Since this thread will not be kept in any rbtree not in a +* list, initialize its list node so that at thread__put() the +* current thread lifetime assuption is kept and we don't segfault +* at list_del_init(). +*/ +INIT_LIST_HEAD (&pt->unknown_thread->node); + err =thread__set_comm (pt->unknown_thread,"unknown" ,0 );if (err) goto err_delete_thread;