From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933452AbcBBPwc (ORCPT ); Tue, 2 Feb 2016 10:52:32 -0500 Received: from mail.kernel.org ([198.145.29.136]:35249 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933268AbcBBPwb (ORCPT ); Tue, 2 Feb 2016 10:52:31 -0500 Date: Tue, 2 Feb 2016 12:52:25 -0300 From: Arnaldo Carvalho de Melo To: Adrian Hunter Cc: Wang Nan , Ingo Molnar , linux-kernel@vger.kernel.org, Wang Nan , Josh Poimboeuf Subject: Re: [PATCH 2/2] perf tools: Fix fault in error patch of intel_pt_process_auxtrace_info() Message-ID: <20160202155225.GD32488@kernel.org> References: <1454346792-24419-1-git-send-email-acme@kernel.org> <1454346792-24419-3-git-send-email-acme@kernel.org> <56B083D3.2000402@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B083D3.2000402@intel.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Feb 02, 2016 at 12:24:19PM +0200, Adrian Hunter escreveu: > 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.: So, can I go with the following patch+description+authorship? >>From 3a4acda1ecbd290973de08250d7dcdfaf5b2fe0f Mon Sep 17 00:00:00 2001 From: Adrian Hunter 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 last thread__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 # Reported-by: Tong Zhang Reported-by: Wang Nan Signed-off-by: Adrian Hunter Tested-by: Arnaldo Carvalho de Melo Cc: Josh Poimboeuf Link: http://lkml.kernel.org/r/1454296865-19749-1-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/intel-pt.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index 81a2eb77ba7f..05d815851be1 100644 --- a/tools/perf/util/intel-pt.c +++ b/tools/perf/util/intel-pt.c @@ -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; -- 2.5.0