From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752954AbcCUIC4 (ORCPT ); Mon, 21 Mar 2016 04:02:56 -0400 Received: from mga02.intel.com ([134.134.136.20]:55385 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751659AbcCUICq (ORCPT ); Mon, 21 Mar 2016 04:02:46 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,370,1455004800"; d="scan'208";a="768049050" From: Alexander Shishkin To: Leon Yu , linux-kernel@vger.kernel.org Cc: Leon Yu , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , stable@vger.kernel.org Subject: Re: [PATCH] perf: fix event leak when perf_event_open() failed to create event_file In-Reply-To: <1458489135-5016-1-git-send-email-chianglungyu@gmail.com> References: <1458489135-5016-1-git-send-email-chianglungyu@gmail.com> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 21 Mar 2016 10:02:42 +0200 Message-ID: <87twk06yxp.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Leon Yu writes: > If something went wrong in anon_inode_getfile, event_file will be set to > non-zero error number and able to bypass the NULL test afterward. > > Consolidate the error path by testing event_file with handly > IS_ERR_OR_NULL() helper since we do want to free event in both cases. > > Signed-off-by: Leon Yu > Fixes: 130056275ade ("perf: Do not double free") > Cc: # v4.5 > --- > kernel/events/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 6146148..5f2e19f 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -8617,7 +8617,7 @@ err_alloc: > * If event_file is set, the fput() above will have called ->release() > * and that will take care of freeing the event. > */ > - if (!event_file) > + if (IS_ERR_OR_NULL(event_file)) > free_event(event); By this time, we have already checked for IS_ERR(event_file) once, why not just fix it up there like so: --- From: Alexander Shishkin Date: Mon, 21 Mar 2016 09:55:09 +0200 Subject: perf: Don't leak event in the syscall error path In the error path, event_file not being NULL is used to determine whether the event itself still needs to be free'd, so fix it up to avoid leaking. Reported-by: Leon Yu Fixes: 130056275ade ("perf: Do not double free") Signed-off-by: Alexander Shishkin --- kernel/events/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index ab83640a50..cc34d55f1e 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -9414,6 +9414,7 @@ SYSCALL_DEFINE5(perf_event_open, f_flags); if (IS_ERR(event_file)) { err = PTR_ERR(event_file); + event_file = NULL; goto err_context; } -- 2.7.0