On Wed, 18 Aug 2010, 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@xxxxxxxxx> FWIW, I was hitting this bug with Ceph a while back and proposed a different fix for it: instead of dropping the lock, my patch did the revalidation under i_mutex. Unfortunately that conflicts with autofs shenanigans and is still sitting in Al's tree awaiting some resolution there, see http://git.kernel.org/?p=linux/kernel/git/viro/vfs-2.6.git;a=commit;h=1f24668c6c673e2e74fb77ff6ef5a07651c3bd10 This approach is simpler and looks okay to me. Reviewed-by: Sage Weil <sage@xxxxxxxxxxxx> sage > > --- > 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); > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
