Re: [patch 01/10] fs: fix do_lookup false negative

[Valerie Aurora <vaurora@redhat.com>]

On Wed, Aug 18, 2010 at 04:37:30AM +1000, Nick Piggin wrote:
> fs: fix do_lookup false negative
> 
> In do_lookup, if we initially find no dentry, we take the directory i_mutex and
> re-check the lookup. If we find a dentry there, then we revalidate it if
> needed. However if that revalidate asks for the dentry to be invalidated, we
> return -ENOENT from do_lookup. What should happen instead is an attempt to
> allocate and lookup a new dentry.
> 
> This is probably not noticed because it is rare. It is only reached if a
> concurrent create races in first (in which case, the dentry probably won't be
> invalidated anyway), or if the racy __d_lookup has failed due to a
> false-negative (which is very rare).
> 
> Fix this by removing code and have it use the normal reval path.
> 
> Signed-off-by: Nick Piggin <npiggin@kernel.dk>
> 
> ---
>  fs/namei.c |   10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> Index: linux-2.6/fs/namei.c
> ===================================================================
> --- linux-2.6.orig/fs/namei.c	2010-08-18 04:04:18.000000000 +1000
> +++ linux-2.6/fs/namei.c	2010-08-18 04:05:15.000000000 +1000
> @@ -709,6 +709,7 @@ static int do_lookup(struct nameidata *n
>  	dentry = __d_lookup(nd->path.dentry, name);
>  	if (!dentry)
>  		goto need_lookup;
> +found:
>  	if (dentry->d_op && dentry->d_op->d_revalidate)
>  		goto need_revalidate;
>  done:
> @@ -766,14 +767,7 @@ out_unlock:
>  	 * we waited on the semaphore. Need to revalidate.
>  	 */
>  	mutex_unlock(&dir->i_mutex);
> -	if (dentry->d_op && dentry->d_op->d_revalidate) {
> -		dentry = do_revalidate(dentry, nd);
> -		if (!dentry)
> -			dentry = ERR_PTR(-ENOENT);
> -	}
> -	if (IS_ERR(dentry))
> -		goto fail;
> -	goto done;
> +	goto found;
>  
>  need_revalidate:
>  	dentry = do_revalidate(dentry, nd);
> 
> 

Looks good.

Reviewed-by: Valerie Aurora <vaurora@redhat.com>

-VAL