On Mon, 2011-09-26 at 16:26 -0700, Linus Torvalds wrote:
> On Mon, Sep 26, 2011 at 4:09 PM, Trond Myklebust
> <Trond.Myklebust@xxxxxxxxxx> wrote:
> >
> > I assume that means that we can get rid of LOOKUP_NO_AUTOMOUNT, and just
> > replace the convoluted logic in follow_automount() with a test for
> > LOOKUP_AUTOMOUNT? If we have agreed on a default behaviour, then that
> > would seem cleaner than enumerating all these exceptions.
>
> I do think that LOOKUP_NO_AUTOMOUNT is bogus. It's not a
> kernel-internal flag, it's for that silly "vfs_fstatat()" interface,
> that was designed when LOOKUP_FOLLOW would cause an automount.
>
> Now that LOOKUP_FOLLOW doesn't do that, the whole reason for
> LOOKUP_NO_AUTOMOUNT really goes away, but then we have that crazy user
> interface that was encoded for it (only for the very special vfs
>
> The question then becomes: should we just ignore (and deprecate) that
> crazy AT_NO_AUTOMOUNT flag? I say "yup".
>
> Or should we do something *really* crazy, and say "if you *don't* have
> AT_SYMLINK_NOFOLLOW *and* you don't have AT_NO_AUTOMOUNT, then we make
> fstatat64() follow automounts, even though no sane version of *stat()
> does so any more"?
>
> Quite frankly, that just sounds insane. But we could do it, and it
> would approximate the old semantics for that system call. Even if
> those old semantics were clearly not very good, and the whole point of
> AT_NO_AUTOMOUNT was to work around problems people must have had..
>
> So my gut feel is that we should just ignore it for now.
> AT_NO_AUTOMOUNT becomes a no-op, and the LOOKUP_NO_AUTOMOUNT flag is
> already effectively one (since nothing but vfs_fstatat sets it, and
> vfs_fstatat now doesn't follow automounts *anyway*)
>
> The "let's just see if anybody complains when we clean stuff up"
> approach, in other words. Because I bet any user who doesn't have
> AT_NO_AUTOMOUNT isn't because they didn't really want an automount,
> it's because they weren't even aware of the issue.
OK. Then how about something along the lines of the following patch
(compile tested only)?
Cheers
Trond
8<----------------------------------------------------------------------------------
>From df2a725b1ae039988feaaceb75c24c8c388cb90e Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
Date: Mon, 26 Sep 2011 20:36:09 -0400
Subject: [PATCH] VFS: Fix the remaining automounter semantics regressions
The concensus seems to be that system calls such as stat() etc should not
trigger an automount. Neither should the l* versions.
This patch therefore adds a LOOKUP_AUTOMOUNT flag to tag those lookups that
_should_ trigger an automount on the last path element.
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
---
fs/cachefiles/bind.c | 3 ++-
fs/configfs/symlink.c | 3 ++-
fs/ecryptfs/main.c | 3 ++-
fs/namei.c | 26 ++++++--------------------
fs/namespace.c | 2 +-
fs/nfs/super.c | 2 +-
fs/notify/fanotify/fanotify_user.c | 2 +-
fs/notify/inotify/inotify_user.c | 2 +-
fs/open.c | 6 +++---
fs/quota/quota.c | 3 ++-
fs/stat.c | 2 --
include/linux/namei.h | 5 +++--
12 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
index 622f469..3ccc141 100644
--- a/fs/cachefiles/bind.c
+++ b/fs/cachefiles/bind.c
@@ -114,7 +114,8 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache)
_debug("- fsdef %p", fsdef);
/* look up the directory at the root of the cache */
- ret = kern_path(cache->rootdirname, LOOKUP_DIRECTORY, &path);
+ ret = kern_path(cache->rootdirname, LOOKUP_DIRECTORY |
+ LOOKUP_AUTOMOUNT, &path);
if (ret < 0)
goto error_open_root;
diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index 0f3eb41..5e1e1d4 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -114,7 +114,8 @@ static int get_target(const char *symname, struct path *path,
{
int ret;
- ret = kern_path(symname, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, path);
+ ret = kern_path(symname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY |
+ LOOKUP_AUTOMOUNT, path);
if (!ret) {
if (path->dentry->d_sb == configfs_sb) {
*target = configfs_get_config_item(path->dentry);
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index b4a6bef..e3de2c8 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -519,7 +519,8 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
s->s_d_op = &ecryptfs_dops;
err = "Reading sb failed";
- rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &path);
+ rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY |
+ LOOKUP_AUTOMOUNT, &path);
if (rc) {
ecryptfs_printk(KERN_WARNING, "kern_path() failed\n");
goto out1;
diff --git a/fs/namei.c b/fs/namei.c
index f478836..5b1608f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -724,25 +724,9 @@ static int follow_automount(struct path *path, unsigned flags,
/* We don't want to mount if someone supplied AT_NO_AUTOMOUNT
* and this is the terminal part of the path.
*/
- if ((flags & LOOKUP_NO_AUTOMOUNT) && !(flags & LOOKUP_PARENT))
+ if (!(flags & LOOKUP_AUTOMOUNT) && !(flags & LOOKUP_PARENT))
return -EISDIR; /* we actually want to stop here */
- /* We don't want to mount if someone's just doing a stat -
- * unless they're stat'ing a directory and appended a '/' to
- * the name.
- *
- * We do, however, want to mount if someone wants to open or
- * create a file of any type under the mountpoint, wants to
- * traverse through the mountpoint or wants to open the
- * mounted directory. Also, autofs may mark negative dentries
- * as being automount points. These will need the attentions
- * of the daemon to instantiate them before they can be used.
- */
- if (!(flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY |
- LOOKUP_OPEN | LOOKUP_CREATE)) &&
- path->dentry->d_inode)
- return -EISDIR;
-
current->total_link_count++;
if (current->total_link_count >= 40)
return -ELOOP;
@@ -1570,7 +1554,8 @@ out_fail:
static inline int lookup_last(struct nameidata *nd, struct path *path)
{
if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])
- nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
+ nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY |
+ LOOKUP_AUTOMOUNT;
nd->flags &= ~LOOKUP_PARENT;
return walk_component(nd, path, &nd->last, nd->last_type,
@@ -2122,7 +2107,8 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
if (!(open_flag & O_CREAT)) {
int symlink_ok = 0;
if (nd->last.name[nd->last.len])
- nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
+ nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY |
+ LOOKUP_AUTOMOUNT;
if (open_flag & O_PATH && !(nd->flags & LOOKUP_FOLLOW))
symlink_ok = 1;
/* we _can_ be in RCU mode here */
@@ -2382,7 +2368,7 @@ struct dentry *kern_path_create(int dfd, const char *pathname, struct path *path
if (nd.last_type != LAST_NORM)
goto out;
nd.flags &= ~LOOKUP_PARENT;
- nd.flags |= LOOKUP_CREATE | LOOKUP_EXCL;
+ nd.flags |= LOOKUP_AUTOMOUNT | LOOKUP_CREATE | LOOKUP_EXCL;
nd.intent.open.flags = O_EXCL;
/*
diff --git a/fs/namespace.c b/fs/namespace.c
index 22bfe82..b4febb2 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1757,7 +1757,7 @@ static int do_loopback(struct path *path, char *old_name,
return err;
if (!old_name || !*old_name)
return -EINVAL;
- err = kern_path(old_name, LOOKUP_FOLLOW, &old_path);
+ err = kern_path(old_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
if (err)
return err;
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 9b7dd70..5b19b6a 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2798,7 +2798,7 @@ static struct dentry *nfs_follow_remote_path(struct vfsmount *root_mnt,
goto out_put_mnt_ns;
ret = vfs_path_lookup(root_mnt->mnt_root, root_mnt,
- export_path, LOOKUP_FOLLOW, &path);
+ export_path, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &path);
nfs_referral_loop_unprotect();
put_mnt_ns(ns_private);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 9fde1c0..c7c8891 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -496,7 +496,7 @@ static int fanotify_find_path(int dfd, const char __user *filename,
if (!(flags & FAN_MARK_DONT_FOLLOW))
lookup_flags |= LOOKUP_FOLLOW;
if (flags & FAN_MARK_ONLYDIR)
- lookup_flags |= LOOKUP_DIRECTORY;
+ lookup_flags |= LOOKUP_DIRECTORY | LOOKUP_AUTOMOUNT;
ret = user_path_at(dfd, filename, lookup_flags, path);
if (ret)
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 8445fbc..145395a 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -774,7 +774,7 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
if (!(mask & IN_DONT_FOLLOW))
flags |= LOOKUP_FOLLOW;
if (mask & IN_ONLYDIR)
- flags |= LOOKUP_DIRECTORY;
+ flags |= LOOKUP_DIRECTORY | LOOKUP_AUTOMOUNT;
ret = inotify_find_inode(pathname, &path, flags);
if (ret)
diff --git a/fs/open.c b/fs/open.c
index f711921..cc38b85 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -918,16 +918,16 @@ static inline int build_open_flags(int flags, int mode, struct open_flags *op)
op->acc_mode = acc_mode;
- op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
+ op->intent = flags & O_PATH ? 0 : (LOOKUP_OPEN | LOOKUP_AUTOMOUNT);
if (flags & O_CREAT) {
- op->intent |= LOOKUP_CREATE;
+ op->intent |= LOOKUP_CREATE|LOOKUP_AUTOMOUNT;
if (flags & O_EXCL)
op->intent |= LOOKUP_EXCL;
}
if (flags & O_DIRECTORY)
- lookup_flags |= LOOKUP_DIRECTORY;
+ lookup_flags |= LOOKUP_DIRECTORY | LOOKUP_AUTOMOUNT;
if (!(flags & O_NOFOLLOW))
lookup_flags |= LOOKUP_FOLLOW;
return lookup_flags;
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index b34bdb2..9ea9fce 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -355,7 +355,8 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
* resolution (think about autofs) and thus deadlocks could arise.
*/
if (cmds == Q_QUOTAON) {
- ret = user_path_at(AT_FDCWD, addr, LOOKUP_FOLLOW, &path);
+ ret = user_path_at(AT_FDCWD, addr, LOOKUP_FOLLOW |
+ LOOKUP_AUTOMOUNT, &path);
if (ret)
pathp = ERR_PTR(ret);
else
diff --git a/fs/stat.c b/fs/stat.c
index ba5316f..78a3aa8 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -81,8 +81,6 @@ int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
if (!(flag & AT_SYMLINK_NOFOLLOW))
lookup_flags |= LOOKUP_FOLLOW;
- if (flag & AT_NO_AUTOMOUNT)
- lookup_flags |= LOOKUP_NO_AUTOMOUNT;
if (flag & AT_EMPTY_PATH)
lookup_flags |= LOOKUP_EMPTY;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 76fe2c6..209ba9f 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -52,7 +52,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
#define LOOKUP_PARENT 0x0010
#define LOOKUP_REVAL 0x0020
#define LOOKUP_RCU 0x0040
-#define LOOKUP_NO_AUTOMOUNT 0x0080
+#define LOOKUP_AUTOMOUNT 0x0080
/*
* Intent data
*/
@@ -70,7 +70,8 @@ extern int user_path_at(int, const char __user *, unsigned, struct path *);
#define user_path(name, path) user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW, path)
#define user_lpath(name, path) user_path_at(AT_FDCWD, name, 0, path)
#define user_path_dir(name, path) \
- user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, path)
+ user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY | \
+ LOOKUP_AUTOMOUNT, path)
extern int kern_path(const char *, unsigned, struct path *);
--
1.7.6.2
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
