Subject: Re: d_instantiate_unique / NFS inode leakage?
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Oleg Drokin <green@linuxhacker.ru>
Cc: linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk
In-Reply-To: <20060105080933.GK5743@linuxhacker.ru>
References: <20060105010047.GJ5743@linuxhacker.ru>
	 <1136424407.7847.37.camel@lade.trondhjem.org>
	 <20060105080933.GK5743@linuxhacker.ru>
Content-Type: text/plain
Date: Fri, 06 Jan 2006 15:07:49 -0500
Message-Id: <1136578069.7843.5.camel@lade.trondhjem.org>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2116
Lines: 56

On Thu, 2006-01-05 at 10:09 +0200, Oleg Drokin wrote:
> Hello!
> 
> On Thu, Jan 05, 2006 at 02:26:47AM +0100, Trond Myklebust wrote:
> > >    Searching for inode leakage in NFS code (seen in 2.6.14 and 2.6.15 at least,
> > >    have not tried earlier versions), I see suspicious place in
> > >    d_instantiate_unique (the only user happens to be NFS).
> > >    There if we have found aliased dentry that we return, inode reference is
> > >    not dropped and inode is not attached anywhere, so it seems the reference
> > >    to inode is leaked in that case.
> > Yep, that looks like it ought to be the correct behaviour. Could you
> > please also add a note to this effect in the DocBook header for
> > d_instantiate_unique?
> 
> Sure.

Looks good to me. ACKed

Cheers,
  Trond

> --- fs/dcache.c.orig	2006-01-05 02:28:57.000000000 +0200
> +++ fs/dcache.c	2006-01-05 10:04:02.000000000 +0200
> @@ -808,10 +808,14 @@ void d_instantiate(struct dentry *entry,
>   *
>   * Fill in inode information in the entry. On success, it returns NULL.
>   * If an unhashed alias of "entry" already exists, then we return the
> - * aliased dentry instead.
> + * aliased dentry instead and drop one reference to inode.
>   *
>   * Note that in order to avoid conflicts with rename() etc, the caller
>   * had better be holding the parent directory semaphore.
> + *
> + * This also assumes that the inode count has been incremented
> + * (or otherwise set) by the caller to indicate that it is now
> + * in use by the dcache.
>   */
>  struct dentry *d_instantiate_unique(struct dentry *entry, struct inode *inode)
>  {
> @@ -838,6 +842,7 @@ struct dentry *d_instantiate_unique(stru
>  		dget_locked(alias);
>  		spin_unlock(&dcache_lock);
>  		BUG_ON(!d_unhashed(alias));
> +		iput(inode);
>  		return alias;
>  	}
>  	list_add(&entry->d_alias, &inode->i_dentry);
> 
> Bye,
>     Oleg

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/