reimplement path_mountpoint() with less magic

... and get rid of a bunch of bugs in it.  Background:
the reason for path_mountpoint() is that umount() really doesn't
want attempts to revalidate the root of what it's trying to umount.
The thing we want to avoid actually happen from complete_walk();
solution was to do something parallel to normal path_lookupat()
and it both went overboard and got the boilerplate subtly
(and not so subtly) wrong.

A better solution is to do pretty much what the normal path_lookupat()
does, but instead of complete_walk() do unlazy_walk().  All it takes
to avoid that ->d_weak_revalidate() call...  mountpoint_last() goes
away, along with everything it got wrong, and so does the magic around
LOOKUP_NO_REVAL.

Another source of bugs is that when we traverse mounts at the final
location (and we need to do that - umount . expects to get whatever's
overmounting ., if any, out of the lookup) we really ought to take
care of ->d_manage() - as it is, manual umount of autofs automount
in progress can lead to unpleasant surprises for the daemon.  Easily
solved by using handle_lookup_down() instead of follow_mount().

Tested-by: Ian Kent <raven@themaw.net>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namei.c b/fs/namei.c
index d6c91d1..204677c 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1649,17 +1649,15 @@ again:
        if (IS_ERR(dentry))
                return dentry;
        if (unlikely(!d_in_lookup(dentry))) {
-               if (!(flags & LOOKUP_NO_REVAL)) {
-                       int error = d_revalidate(dentry, flags);
-                       if (unlikely(error <= 0)) {
-                               if (!error) {
-                                       d_invalidate(dentry);
-                                       dput(dentry);
-                                       goto again;
-                               }
+               int error = d_revalidate(dentry, flags);
+               if (unlikely(error <= 0)) {
+                       if (!error) {
+                               d_invalidate(dentry);
                                dput(dentry);
-                               dentry = ERR_PTR(error);
+                               goto again;
                        }
+                       dput(dentry);
+                       dentry = ERR_PTR(error);
                }
        } else {
                old = inode->i_op->lookup(inode, dentry, flags);
@@ -2617,72 +2615,6 @@ int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
 }
 EXPORT_SYMBOL(user_path_at_empty);
 
-/**
- * mountpoint_last - look up last component for umount
- * @nd:   pathwalk nameidata - currently pointing at parent directory of "last"
- *
- * This is a special lookup_last function just for umount. In this case, we
- * need to resolve the path without doing any revalidation.
- *
- * The nameidata should be the result of doing a LOOKUP_PARENT pathwalk. Since
- * mountpoints are always pinned in the dcache, their ancestors are too. Thus,
- * in almost all cases, this lookup will be served out of the dcache. The only
- * cases where it won't are if nd->last refers to a symlink or the path is
- * bogus and it doesn't exist.
- *
- * Returns:
- * -error: if there was an error during lookup. This includes -ENOENT if the
- *         lookup found a negative dentry.
- *
- * 0:      if we successfully resolved nd->last and found it to not to be a
- *         symlink that needs to be followed.
- *
- * 1:      if we successfully resolved nd->last and found it to be a symlink
- *         that needs to be followed.
- */
-static int
-mountpoint_last(struct nameidata *nd)
-{
-       int error = 0;
-       struct dentry *dir = nd->path.dentry;
-       struct path path;
-
-       /* If we're in rcuwalk, drop out of it to handle last component */
-       if (nd->flags & LOOKUP_RCU) {
-               if (unlazy_walk(nd))
-                       return -ECHILD;
-       }
-
-       nd->flags &= ~LOOKUP_PARENT;
-
-       if (unlikely(nd->last_type != LAST_NORM)) {
-               error = handle_dots(nd, nd->last_type);
-               if (error)
-                       return error;
-               path.dentry = dget(nd->path.dentry);
-       } else {
-               path.dentry = d_lookup(dir, &nd->last);
-               if (!path.dentry) {
-                       /*
-                        * No cached dentry. Mounted dentries are pinned in the
-                        * cache, so that means that this dentry is probably
-                        * a symlink or the path doesn't actually point
-                        * to a mounted dentry.
-                        */
-                       path.dentry = lookup_slow(&nd->last, dir,
-                                            nd->flags | LOOKUP_NO_REVAL);
-                       if (IS_ERR(path.dentry))
-                               return PTR_ERR(path.dentry);
-               }
-       }
-       if (d_flags_negative(smp_load_acquire(&path.dentry->d_flags))) {
-               dput(path.dentry);
-               return -ENOENT;
-       }
-       path.mnt = nd->path.mnt;
-       return step_into(nd, &path, 0, d_backing_inode(path.dentry), 0);
-}
-
 /**
  * path_mountpoint - look up a path to be umounted
  * @nd:                lookup context
@@ -2699,14 +2631,17 @@ path_mountpoint(struct nameidata *nd, unsigned flags, struct path *path)
        int err;
 
        while (!(err = link_path_walk(s, nd)) &&
-               (err = mountpoint_last(nd)) > 0) {
+               (err = lookup_last(nd)) > 0) {
                s = trailing_symlink(nd);
        }
+       if (!err && (nd->flags & LOOKUP_RCU))
+               err = unlazy_walk(nd);
+       if (!err)
+               err = handle_lookup_down(nd);
        if (!err) {
                *path = nd->path;
                nd->path.mnt = NULL;
                nd->path.dentry = NULL;
-               follow_mount(path);
        }
        terminate_walk(nd);
        return err;

diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index f64a33d..2a82dcc 100644 (file)
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -206,7 +206,6 @@ TRACE_DEFINE_ENUM(LOOKUP_AUTOMOUNT);
 TRACE_DEFINE_ENUM(LOOKUP_PARENT);
 TRACE_DEFINE_ENUM(LOOKUP_REVAL);
 TRACE_DEFINE_ENUM(LOOKUP_RCU);
-TRACE_DEFINE_ENUM(LOOKUP_NO_REVAL);
 TRACE_DEFINE_ENUM(LOOKUP_OPEN);
 TRACE_DEFINE_ENUM(LOOKUP_CREATE);
 TRACE_DEFINE_ENUM(LOOKUP_EXCL);
@@ -224,7 +223,6 @@ TRACE_DEFINE_ENUM(LOOKUP_DOWN);
                        { LOOKUP_PARENT, "PARENT" }, \
                        { LOOKUP_REVAL, "REVAL" }, \
                        { LOOKUP_RCU, "RCU" }, \
-                       { LOOKUP_NO_REVAL, "NO_REVAL" }, \
                        { LOOKUP_OPEN, "OPEN" }, \
                        { LOOKUP_CREATE, "CREATE" }, \
                        { LOOKUP_EXCL, "EXCL" }, \

diff --git a/include/linux/namei.h b/include/linux/namei.h
index 7fe7b87..07bfb08 100644 (file)
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -34,7 +34,6 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 
 /* internal use only */
 #define LOOKUP_PARENT          0x0010
-#define LOOKUP_NO_REVAL                0x0080
 #define LOOKUP_JUMPED          0x1000
 #define LOOKUP_ROOT            0x2000
 #define LOOKUP_ROOT_GRABBED    0x0008