Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932105AbdHWOik (ORCPT ); Wed, 23 Aug 2017 10:38:40 -0400 Received: from mail.kernel.org ([198.145.29.99]:59304 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754053AbdHWOij (ORCPT ); Wed, 23 Aug 2017 10:38:39 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 60A9220C48 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Wed, 23 Aug 2017 10:38:36 -0400 From: Steven Rostedt To: Chunyu Hu Cc: mingo@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter Message-ID: <20170823103836.584a5dc5@gandalf.local.home> In-Reply-To: <1502705898-27571-2-git-send-email-chuhu@redhat.com> References: <1502705898-27571-1-git-send-email-chuhu@redhat.com> <1502705898-27571-2-git-send-email-chuhu@redhat.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4696 Lines: 108 On Mon, 14 Aug 2017 18:18:18 +0800 Chunyu Hu wrote: > Found this kmemleak in error path when setting event trigger > filter. The steps is as below. > > cd /sys/kernel/debug/tracing/events/irq/irq_handler_entry > echo 'enable_event:kmem:kmalloc:3 if nr_rq > 1' > trigger > echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger > echo 'enable_event:kmem:kmalloc:3 if irq > ' > trigger > > unreferenced object 0xffffa06e570186a0 (size 32): > comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s) > hex dump (first 32 bytes): > 00 00 00 00 01 00 00 00 00 a6 86 69 6e a0 ff ff ...........in... > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [] kmemleak_alloc+0x4a/0xa0 > [] kmem_cache_alloc_trace+0xca/0x1d0 > [] create_filter_start.constprop.28+0x42/0x730 > [] create_filter+0x4a/0xc0 > [] create_event_filter+0xc/0x10 > [] set_trigger_filter+0x84/0x130 > [] event_enable_trigger_func+0x25f/0x340 > [] event_trigger_write+0xfd/0x1a0 > [] __vfs_write+0x37/0x170 > [] vfs_write+0xb2/0x1b0 > [] SyS_write+0x55/0xc0 > [] do_syscall_64+0x67/0x150 > [] return_from_SYSCALL_64+0x0/0x6a > [] 0xffffffffffffffff > unreferenced object 0xffffa06e6986a600 (size 512): > comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s) > hex dump (first 32 bytes): > b0 59 f8 96 ff ff ff ff 00 00 00 00 00 00 00 00 .Y.............. > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [] kmemleak_alloc+0x4a/0xa0 > [] __kmalloc+0xe8/0x220 > [] replace_preds+0x4c8/0x970 > [] create_filter+0x91/0xc0 > [] create_event_filter+0xc/0x10 > [] set_trigger_filter+0x84/0x130 > [] event_enable_trigger_func+0x25f/0x340 > [] event_trigger_write+0xfd/0x1a0 > [] __vfs_write+0x37/0x170 > [] vfs_write+0xb2/0x1b0 > [] SyS_write+0x55/0xc0 > [] do_syscall_64+0x67/0x150 > [] return_from_SYSCALL_64+0x0/0x6a > [] 0xffffffffffffffff > unreferenced object 0xffffa06e718a7560 (size 32): > comm "bash", pid 2521, jiffies 4335807465 (age 43861.320s) > hex dump (first 32 bytes): > 00 00 00 00 01 00 00 00 00 a2 76 5b 6e a0 ff ff ..........v[n... > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [] kmemleak_alloc+0x4a/0xa0 > [] kmem_cache_alloc_trace+0xca/0x1d0 > [] create_filter_start.constprop.28+0x42/0x730 > [] create_filter+0x4a/0xc0 > [] create_event_filter+0xc/0x10 > [] set_trigger_filter+0x84/0x130 > [] event_enable_trigger_func+0x25f/0x340 > [] event_trigger_write+0xfd/0x1a0 > [] __vfs_write+0x37/0x170 > [] vfs_write+0xb2/0x1b0 > [] SyS_write+0x55/0xc0 > [] do_syscall_64+0x67/0x150 > [] return_from_SYSCALL_64+0x0/0x6a > [] 0xffffffffffffffff > > Signed-off-by: Chunyu Hu > --- > kernel/trace/trace_events_trigger.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c > index f2ac9d4..955c3eb 100644 > --- a/kernel/trace/trace_events_trigger.c > +++ b/kernel/trace/trace_events_trigger.c > @@ -739,7 +739,7 @@ int set_trigger_filter(char *filter_str, > /* The filter is for the 'trigger' event, not the triggered event */ > ret = create_event_filter(file->event_call, filter_str, false, &filter); The filter is allocated by create_event_filter. If that returns a failure, then that should be the one to free it. It is bad taste to require the calling function to require it. -- Steve > if (ret) > - goto out; > + goto out_free; > assign: > tmp = rcu_access_pointer(data->filter); > > @@ -764,6 +764,10 @@ int set_trigger_filter(char *filter_str, > } > out: > return ret; > + > + out_free: > + free_event_filter(filter); > + goto out; > } > > static LIST_HEAD(named_triggers);