Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932405AbdHWQwz convert rfc822-to-8bit (ORCPT ); Wed, 23 Aug 2017 12:52:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:42540 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932260AbdHWQww (ORCPT ); Wed, 23 Aug 2017 12:52:52 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4A10920C48 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 12:52:49 -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: <20170823125249.102d0908@gandalf.local.home> In-Reply-To: <20170823104155.27f7c6f6@gandalf.local.home> References: <1502705898-27571-1-git-send-email-chuhu@redhat.com> <1502705898-27571-2-git-send-email-chuhu@redhat.com> <20170823103836.584a5dc5@gandalf.local.home> <20170823104155.27f7c6f6@gandalf.local.home> 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: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3370 Lines: 87 On Wed, 23 Aug 2017 10:41:55 -0400 Steven Rostedt wrote: > * On success, returns 0 and *@filterp points to the new filter. On > * failure, returns -errno and *@filterp may point to %NULL or to a new > * filter. In the latter case, the returned filter contains error > * information if @set_str is %true and the caller is responsible for > * freeing it. > > So filter contains an error string when it fails. It seems that we > should somehow propagate that up the chain to display. I'll look more > into this. The bug is in create_filter(), because "set_str" is set to false, and the filter should not be passed back allocated on error. The correct fix is below. Thanks! -- Steve >From 9975be0b2dccaea8ec3574d69a3504e11659a6ea Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (VMware)" Date: Wed, 23 Aug 2017 12:46:27 -0400 Subject: [PATCH] tracing: Fix freeing of filter in create_filter() when set_str is false Performing the following task with kmemleak enabled: # cd /sys/kernel/tracing/events/irq/irq_handler_entry/ # echo 'enable_event:kmem:kmalloc:3 if irq >' > trigger # echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger # echo scan > /sys/kernel/debug/kmemleak # cat /sys/kernel/debug/kmemleak unreferenced object 0xffff8800b9290308 (size 32): comm "bash", pid 1114, jiffies 4294848451 (age 141.139s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 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+0x158/0x290 [] create_filter_start.constprop.28+0x99/0x940 [] create_filter+0xa9/0x160 [] create_event_filter+0xc/0x10 [] set_trigger_filter+0xe5/0x210 [] event_enable_trigger_func+0x324/0x490 [] event_trigger_write+0x1a2/0x260 [] __vfs_write+0xd7/0x380 [] vfs_write+0x101/0x260 [] SyS_write+0xab/0x130 [] entry_SYSCALL_64_fastpath+0x1f/0xbe [] 0xffffffffffffffff The function create_filter() is passed a 'filterp' pointer that gets allocated, and if "set_str" is true, it is up to the caller to free it, even on error. The problem is that the pointer is not freed by create_filter() when set_str is false. This is a bug, and it is not up to the caller to free the filter on error if it doesn't care about the string. Link: http://lkml.kernel.org/r/1502705898-27571-2-git-send-email-chuhu@redhat.com Reported-by: Chunyu Hu Signed-off-by: Steven Rostedt (VMware) --- kernel/trace/trace_events_filter.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index 59a411f..181e139 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c @@ -1959,6 +1959,10 @@ static int create_filter(struct trace_event_call *call, if (err && set_str) append_filter_err(ps, filter); } + if (err && !set_str) { + free_event_filter(filter); + filter = NULL; + } create_filter_finish(ps); *filterp = filter; -- 2.9.5