From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932731AbbBBHzJ (ORCPT ); Mon, 2 Feb 2015 02:55:09 -0500 Received: from mail-pa0-f45.google.com ([209.85.220.45]:54628 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932551AbbBBHzG (ORCPT ); Mon, 2 Feb 2015 02:55:06 -0500 Date: Sun, 1 Feb 2015 23:55:02 -0800 From: Omar Sandoval To: Christoph Hellwig Cc: Alexander Viro , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] posix_acl: fix reference leaks in posix_acl_create Message-ID: <20150202075502.GA24678@mew> References: <0d8672e771b8fc1fef94db4b40b418afc8763c2b.1422338890.git.osandov@osandov.com> <20150128170952.GB17528@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150128170952.GB17528@lst.de> 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 On Wed, Jan 28, 2015 at 06:09:52PM +0100, Christoph Hellwig wrote: > On Mon, Jan 26, 2015 at 10:16:53PM -0800, Omar Sandoval wrote: > > get_acl gets a reference which we must release in the error cases. > > > > Signed-off-by: Omar Sandoval > > Looks good, but at this point goto-based unwinding might be in order. Hi, Christoph, There are already a couple of return paths in posix_acl_create, and there are only these two error cases, so I think gotos might actually make the code more confusing. In any case, here's an idea: posix_acl: fix reference leaks in posix_acl_create get_acl gets a reference which we must release in the error cases. Signed-off-by: Omar Sandoval diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 0855f77..515d315 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -564,13 +564,11 @@ posix_acl_create(struct inode *dir, umode_t *mode, *acl = posix_acl_clone(p, GFP_NOFS); if (!*acl) - return -ENOMEM; + goto no_mem; ret = posix_acl_create_masq(*acl, mode); - if (ret < 0) { - posix_acl_release(*acl); - return -ENOMEM; - } + if (ret < 0) + goto no_mem_clone; if (ret == 0) { posix_acl_release(*acl); @@ -591,6 +589,12 @@ no_acl: *default_acl = NULL; *acl = NULL; return 0; + +no_mem_clone: + posix_acl_release(*acl); +no_mem: + posix_acl_release(p); + return -ENOMEM; } EXPORT_SYMBOL_GPL(posix_acl_create); -- Omar