This series contains several patches that resulted mostly from comments
made by Al Viro (quite a long time ago now).
They are:
- make use of the .permission() method for access checks.
- use dentry info count instead of simple_empty() to avoid
taking a spinlock.
- add a use cases comment to autofs_mountpoint_changed().
- make better use of mount trigger flags (I think this was
actually mentioned by me at the time).
Others:
- fix usage consistency problem with dentry info count.
- remove unused inode field from info struct.
Signed-off-by: Ian Kent <[email protected]>
---
Ian Kent (6):
autofs: use inode permission method for write access
autofs: make dentry info count consistent
autofs: use dentry info count instead of simple_empty()
autofs: add comment about autofs_mountpoint_changed()
autofs: remove unused ino field inode
autofs: manage dentry info mount trigger flags better
fs/autofs/autofs_i.h | 7 +-
fs/autofs/expire.c | 2 +-
fs/autofs/inode.c | 1 +
fs/autofs/root.c | 203 +++++++++++++++++++------------------------
4 files changed, 98 insertions(+), 115 deletions(-)
--
Ian Kent
Eliminate some code duplication from mkdir/rmdir/symlink/unlink
methods by using the inode operation .permission().
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs/root.c | 63 +++++++++++++++++++-----------------------------------
1 file changed, 22 insertions(+), 41 deletions(-)
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 91fe4548c256..fef6ed991022 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -10,6 +10,7 @@
#include "autofs_i.h"
+static int autofs_dir_permission(struct user_namespace *, struct inode *, int);
static int autofs_dir_symlink(struct user_namespace *, struct inode *,
struct dentry *, const char *);
static int autofs_dir_unlink(struct inode *, struct dentry *);
@@ -50,6 +51,7 @@ const struct file_operations autofs_dir_operations = {
const struct inode_operations autofs_dir_inode_operations = {
.lookup = autofs_lookup,
+ .permission = autofs_dir_permission,
.unlink = autofs_dir_unlink,
.symlink = autofs_dir_symlink,
.mkdir = autofs_dir_mkdir,
@@ -526,11 +528,30 @@ static struct dentry *autofs_lookup(struct inode *dir,
return NULL;
}
+static int autofs_dir_permission(struct user_namespace *mnt_userns,
+ struct inode *inode, int mask)
+{
+ if (mask & MAY_WRITE) {
+ struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
+
+ if (!autofs_oz_mode(sbi))
+ return -EACCES;
+
+ /* autofs_oz_mode() needs to allow path walks when the
+ * autofs mount is catatonic but the state of an autofs
+ * file system needs to be preserved over restarts.
+ */
+ if (sbi->flags & AUTOFS_SBI_CATATONIC)
+ return -EACCES;
+ }
+
+ return generic_permission(mnt_userns, inode, mask);
+}
+
static int autofs_dir_symlink(struct user_namespace *mnt_userns,
struct inode *dir, struct dentry *dentry,
const char *symname)
{
- struct autofs_sb_info *sbi = autofs_sbi(dir->i_sb);
struct autofs_info *ino = autofs_dentry_ino(dentry);
struct autofs_info *p_ino;
struct inode *inode;
@@ -539,16 +560,6 @@ static int autofs_dir_symlink(struct user_namespace *mnt_userns,
pr_debug("%s <- %pd\n", symname, dentry);
- if (!autofs_oz_mode(sbi))
- return -EACCES;
-
- /* autofs_oz_mode() needs to allow path walks when the
- * autofs mount is catatonic but the state of an autofs
- * file system needs to be preserved over restarts.
- */
- if (sbi->flags & AUTOFS_SBI_CATATONIC)
- return -EACCES;
-
BUG_ON(!ino);
autofs_clean_ino(ino);
@@ -601,16 +612,6 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
struct autofs_info *ino = autofs_dentry_ino(dentry);
struct autofs_info *p_ino;
- if (!autofs_oz_mode(sbi))
- return -EACCES;
-
- /* autofs_oz_mode() needs to allow path walks when the
- * autofs mount is catatonic but the state of an autofs
- * file system needs to be preserved over restarts.
- */
- if (sbi->flags & AUTOFS_SBI_CATATONIC)
- return -EACCES;
-
ino->count--;
p_ino = autofs_dentry_ino(dentry->d_parent);
p_ino->count--;
@@ -683,16 +684,6 @@ static int autofs_dir_rmdir(struct inode *dir, struct dentry *dentry)
pr_debug("dentry %p, removing %pd\n", dentry, dentry);
- if (!autofs_oz_mode(sbi))
- return -EACCES;
-
- /* autofs_oz_mode() needs to allow path walks when the
- * autofs mount is catatonic but the state of an autofs
- * file system needs to be preserved over restarts.
- */
- if (sbi->flags & AUTOFS_SBI_CATATONIC)
- return -EACCES;
-
if (ino->count != 1)
return -ENOTEMPTY;
@@ -726,16 +717,6 @@ static int autofs_dir_mkdir(struct user_namespace *mnt_userns,
struct autofs_info *p_ino;
struct inode *inode;
- if (!autofs_oz_mode(sbi))
- return -EACCES;
-
- /* autofs_oz_mode() needs to allow path walks when the
- * autofs mount is catatonic but the state of an autofs
- * file system needs to be preserved over restarts.
- */
- if (sbi->flags & AUTOFS_SBI_CATATONIC)
- return -EACCES;
-
pr_debug("dentry %p, creating %pd\n", dentry, dentry);
BUG_ON(!ino);
The function autofs_mountpoint_changed() is unusual, add a comment
about two cases for which it is needed.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs/root.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index e0fa71eb5c05..ca03c1cae2be 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -291,9 +291,26 @@ static struct dentry *autofs_mountpoint_changed(struct path *path)
struct dentry *dentry = path->dentry;
struct autofs_sb_info *sbi = autofs_sbi(dentry->d_sb);
- /*
- * If this is an indirect mount the dentry could have gone away
- * as a result of an expire and a new one created.
+ /* If this is an indirect mount the dentry could have gone away
+ * and a new one created.
+ *
+ * This is unusual and I can't remember the case for which it
+ * was originally added now. But an example of how this can
+ * happen is an autofs indirect mount that has the "browse"
+ * option set and also has the "symlink" option in the autofs
+ * map entry. In this case the daemon will remove the browse
+ * directory and create a symlink as the mount leaving the
+ * struct path stale.
+ *
+ * Another not so obvious case is when a mount in an autofs
+ * indirect mount that uses the "nobrowse" option is being
+ * expired at the same time as a path walk. If the mount has
+ * been umounted but the mount point directory seen before
+ * becoming unhashed (during a lockless path walk) when a stat
+ * family system call is made the mount won't be re-mounted as
+ * it should. In this case the mount point that's been removed
+ * (by the daemon) will be stale and the a new mount point
+ * dentry created.
*/
if (autofs_type_indirect(sbi->type) && d_unhashed(dentry)) {
struct dentry *parent = dentry->d_parent;
Remove the unused inode field of the autofs dentry info
structure.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs/autofs_i.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 0117d6e06300..d5a44fa88acf 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -51,8 +51,6 @@ extern struct file_system_type autofs_fs_type;
*/
struct autofs_info {
struct dentry *dentry;
- struct inode *inode;
-
int flags;
struct completion expire_complete;
The dentry info. field count is used to check if a dentry is in use
during expire. But, to be used for this the count field must account
for the presence of child dentries in a directory dentry.
Therefore it can also be used to check for an empty directory dentry
which can be done without having to to take an additional lock or
account for the presence of a readdir cursor dentry as is done by
simple_empty().
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs/autofs_i.h | 5 +++++
fs/autofs/expire.c | 2 +-
fs/autofs/root.c | 18 ++++++++----------
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 918826eaceea..0117d6e06300 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -148,6 +148,11 @@ static inline int autofs_oz_mode(struct autofs_sb_info *sbi)
task_pgrp(current) == sbi->oz_pgrp);
}
+static inline bool autofs_empty(struct autofs_info *ino)
+{
+ return ino->count < 2;
+}
+
struct inode *autofs_get_inode(struct super_block *, umode_t);
void autofs_free_ino(struct autofs_info *);
diff --git a/fs/autofs/expire.c b/fs/autofs/expire.c
index b3fefd6237c3..038b3d2d9f57 100644
--- a/fs/autofs/expire.c
+++ b/fs/autofs/expire.c
@@ -371,7 +371,7 @@ static struct dentry *should_expire(struct dentry *dentry,
return NULL;
}
- if (simple_empty(dentry))
+ if (autofs_empty(ino))
return NULL;
/* Case 2: tree mount, expire iff entire tree is not busy */
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 442d27d9cb1b..e0fa71eb5c05 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -79,6 +79,7 @@ static int autofs_dir_open(struct inode *inode, struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct autofs_sb_info *sbi = autofs_sbi(dentry->d_sb);
+ struct autofs_info *ino = autofs_dentry_ino(dentry);
pr_debug("file=%p dentry=%p %pd\n", file, dentry, dentry);
@@ -95,7 +96,7 @@ static int autofs_dir_open(struct inode *inode, struct file *file)
* it.
*/
spin_lock(&sbi->lookup_lock);
- if (!path_is_mountpoint(&file->f_path) && simple_empty(dentry)) {
+ if (!path_is_mountpoint(&file->f_path) && autofs_empty(ino)) {
spin_unlock(&sbi->lookup_lock);
return -ENOENT;
}
@@ -364,7 +365,7 @@ static struct vfsmount *autofs_d_automount(struct path *path)
* the mount never trigger mounts themselves (they have an
* autofs trigger mount mounted on them). But v4 pseudo direct
* mounts do need the leaves to trigger mounts. In this case
- * we have no choice but to use the list_empty() check and
+ * we have no choice but to use the autofs_empty() check and
* require user space behave.
*/
if (sbi->version > 4) {
@@ -373,7 +374,7 @@ static struct vfsmount *autofs_d_automount(struct path *path)
goto done;
}
} else {
- if (!simple_empty(dentry)) {
+ if (!autofs_empty(ino)) {
spin_unlock(&sbi->fs_lock);
goto done;
}
@@ -428,9 +429,8 @@ static int autofs_d_manage(const struct path *path, bool rcu_walk)
if (rcu_walk) {
/* We don't need fs_lock in rcu_walk mode,
- * just testing 'AUTOFS_INFO_NO_RCU' is enough.
- * simple_empty() takes a spinlock, so leave it
- * to last.
+ * just testing 'AUTOFS_INF_WANT_EXPIRE' is enough.
+ *
* We only return -EISDIR when certain this isn't
* a mount-trap.
*/
@@ -443,9 +443,7 @@ static int autofs_d_manage(const struct path *path, bool rcu_walk)
inode = d_inode_rcu(dentry);
if (inode && S_ISLNK(inode->i_mode))
return -EISDIR;
- if (list_empty(&dentry->d_subdirs))
- return 0;
- if (!simple_empty(dentry))
+ if (!autofs_empty(ino))
return -EISDIR;
return 0;
}
@@ -465,7 +463,7 @@ static int autofs_d_manage(const struct path *path, bool rcu_walk)
* we can avoid needless calls ->d_automount() and avoid
* an incorrect ELOOP error return.
*/
- if ((!path_is_mountpoint(path) && !simple_empty(dentry)) ||
+ if ((!path_is_mountpoint(path) && !autofs_empty(ino)) ||
(d_really_is_positive(dentry) && d_is_symlink(dentry)))
status = -EISDIR;
}
The autofs managed dentry flags are left set for dentries in an
autofs mount directory regardless of whether the dentry should
trigger a mount (a non-empty directory or a symlink doesn't).
But properly managing these flags can sometimes provide the loop
termination condition needed when following mounts which now uses
an -EISDIR return.
Signed-off-by: Ian Kent <[email protected]>
---
fs/autofs/root.c | 97 +++++++++++++++++++++++-------------------------------
1 file changed, 42 insertions(+), 55 deletions(-)
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index ca03c1cae2be..d140d06c5bc6 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -529,9 +529,8 @@ static struct dentry *autofs_lookup(struct inode *dir,
spin_lock(&sbi->lookup_lock);
spin_lock(&dentry->d_lock);
- /* Mark entries in the root as mount triggers */
- if (IS_ROOT(dentry->d_parent) &&
- autofs_type_indirect(sbi->type))
+ /* Mark indirect mount entries as mount triggers */
+ if (autofs_type_indirect(sbi->type))
__managed_dentry_set_managed(dentry);
dentry->d_fsdata = ino;
ino->dentry = dentry;
@@ -567,7 +566,9 @@ static int autofs_dir_symlink(struct user_namespace *mnt_userns,
struct inode *dir, struct dentry *dentry,
const char *symname)
{
+ struct autofs_sb_info *sbi = autofs_sbi(dir->i_sb);
struct autofs_info *ino = autofs_dentry_ino(dentry);
+ struct dentry *parent = dentry->d_parent;
struct autofs_info *p_ino;
struct inode *inode;
size_t size = strlen(symname);
@@ -602,6 +603,16 @@ static int autofs_dir_symlink(struct user_namespace *mnt_userns,
dir->i_mtime = current_time(dir);
+ /* Symlinks don't trigger mounts */
+ managed_dentry_clear_managed(dentry);
+ /* Clear containing directory flags if it's no longer empty */
+ if (autofs_dentry_ino(parent)->count == 2) {
+ /* Don't set or clear type indirect root */
+ if (!IS_ROOT(parent) ||
+ !autofs_type_indirect(sbi->type))
+ managed_dentry_clear_managed(parent);
+ }
+
return 0;
}
@@ -624,12 +635,21 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
{
struct autofs_sb_info *sbi = autofs_sbi(dir->i_sb);
struct autofs_info *ino = autofs_dentry_ino(dentry);
+ struct dentry *parent = dentry->d_parent;
struct autofs_info *p_ino;
p_ino = autofs_dentry_ino(dentry->d_parent);
p_ino->count--;
dput(ino->dentry);
+ /* Set containing directory flags if it's now empty */
+ if (autofs_dentry_ino(parent)->count == 1) {
+ /* Don't set or clear type indirect root */
+ if (!IS_ROOT(parent) ||
+ !autofs_type_indirect(sbi->type))
+ managed_dentry_set_managed(parent);
+ }
+
d_inode(dentry)->i_size = 0;
clear_nlink(d_inode(dentry));
@@ -643,56 +663,11 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
return 0;
}
-/*
- * Version 4 of autofs provides a pseudo direct mount implementation
- * that relies on directories at the leaves of a directory tree under
- * an indirect mount to trigger mounts. To allow for this we need to
- * set the DMANAGED_AUTOMOUNT and DMANAGED_TRANSIT flags on the leaves
- * of the directory tree. There is no need to clear the automount flag
- * following a mount or restore it after an expire because these mounts
- * are always covered. However, it is necessary to ensure that these
- * flags are clear on non-empty directories to avoid unnecessary calls
- * during path walks.
- */
-static void autofs_set_leaf_automount_flags(struct dentry *dentry)
-{
- struct dentry *parent;
-
- /* root and dentrys in the root are already handled */
- if (IS_ROOT(dentry->d_parent))
- return;
-
- managed_dentry_set_managed(dentry);
-
- parent = dentry->d_parent;
- /* only consider parents below dentrys in the root */
- if (IS_ROOT(parent->d_parent))
- return;
- managed_dentry_clear_managed(parent);
-}
-
-static void autofs_clear_leaf_automount_flags(struct dentry *dentry)
-{
- struct dentry *parent;
-
- /* flags for dentrys in the root are handled elsewhere */
- if (IS_ROOT(dentry->d_parent))
- return;
-
- managed_dentry_clear_managed(dentry);
-
- parent = dentry->d_parent;
- /* only consider parents below dentrys in the root */
- if (IS_ROOT(parent->d_parent))
- return;
- if (autofs_dentry_ino(parent)->count == 2)
- managed_dentry_set_managed(parent);
-}
-
static int autofs_dir_rmdir(struct inode *dir, struct dentry *dentry)
{
struct autofs_sb_info *sbi = autofs_sbi(dir->i_sb);
struct autofs_info *ino = autofs_dentry_ino(dentry);
+ struct dentry *parent = dentry->d_parent;
struct autofs_info *p_ino;
pr_debug("dentry %p, removing %pd\n", dentry, dentry);
@@ -705,15 +680,20 @@ static int autofs_dir_rmdir(struct inode *dir, struct dentry *dentry)
d_drop(dentry);
spin_unlock(&sbi->lookup_lock);
- if (sbi->version < 5)
- autofs_clear_leaf_automount_flags(dentry);
-
p_ino = autofs_dentry_ino(dentry->d_parent);
p_ino->count--;
dput(ino->dentry);
d_inode(dentry)->i_size = 0;
clear_nlink(d_inode(dentry));
+ /* Set containing directory flags if it's now empty */
+ if (autofs_dentry_ino(parent)->count == 1) {
+ /* Don't set or clear type indirect root */
+ if (!IS_ROOT(parent) ||
+ !autofs_type_indirect(sbi->type))
+ managed_dentry_set_managed(parent);
+ }
+
if (dir->i_nlink)
drop_nlink(dir);
@@ -726,6 +706,7 @@ static int autofs_dir_mkdir(struct user_namespace *mnt_userns,
{
struct autofs_sb_info *sbi = autofs_sbi(dir->i_sb);
struct autofs_info *ino = autofs_dentry_ino(dentry);
+ struct dentry *parent = dentry->d_parent;
struct autofs_info *p_ino;
struct inode *inode;
@@ -742,15 +723,21 @@ static int autofs_dir_mkdir(struct user_namespace *mnt_userns,
return -ENOMEM;
d_add(dentry, inode);
- if (sbi->version < 5)
- autofs_set_leaf_automount_flags(dentry);
-
dget(dentry);
p_ino = autofs_dentry_ino(dentry->d_parent);
p_ino->count++;
inc_nlink(dir);
dir->i_mtime = current_time(dir);
+ managed_dentry_set_managed(dentry);
+ /* Clear containing directory flags if it's no longer empty */
+ if (autofs_dentry_ino(parent)->count == 2) {
+ /* Don't set or clear type indirect root */
+ if (!IS_ROOT(parent) ||
+ !autofs_type_indirect(sbi->type))
+ managed_dentry_clear_managed(parent);
+ }
+
return 0;
}