Date: Mon, 27 Mar 2017 17:52:27 -0700
From: vcaputo@pengaru.com
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: hughd@google.com, linux-kernel <linux-kernel@vger.kernel.org>,
        linux-mm@kvack.org
Subject: Re: [PATCH] shmem: fix __shmem_file_setup error path leaks
Message-ID: <20170328005227.GW802@shells.gnugeneration.com>
References: <20170327170534.GA16903@shells.gnugeneration.com>
 <20170327212127.GF29622@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170327212127.GF29622@ZenIV.linux.org.uk>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1080
Lines: 26

On Mon, Mar 27, 2017 at 10:21:27PM +0100, Al Viro wrote:
> On Mon, Mar 27, 2017 at 10:05:34AM -0700, Vito Caputo wrote:
> > The existing path and memory cleanups appear to be in reverse order, and
> > there's no iput() potentially leaking the inode in the last two error gotos.
> > 
> > Also make put_memory shmem_unacct_size() conditional on !inode since if we
> > entered cleanup at put_inode, shmem_evict_inode() occurs via
> > iput()->iput_final(), which performs the shmem_unacct_size() for us.
> > 
> > Signed-off-by: Vito Caputo <vcaputo@pengaru.com>
> > ---
> > 
> > This caught my eye while looking through the memfd_create() implementation.
> > Included patch was compile tested only...
> 
> Obviously so, since you've just introduced a double iput() there.  After
>         d_instantiate(path.dentry, inode);
> dropping the reference to path.dentry (done by path_put(&path)) will drop
> the reference to inode transferred into that dentry by d_instantiate().
> NAK.

I see, so it's correct as-is, thanks for the review and apologies for the
noise!

Cheers,
Vito Caputo