On Fri, Apr 08, 2016 at 08:51:03AM +0800, Ian Kent wrote:
[snip]
>
> I think this is a better approach (beware, not even compile tested so it
> almost certainly doesn't work properly) and I'm not even sure that
> exposing __is_local_mountpoint() is ok from a VFS POV but something
> needs to be done:
>
> autofs4 - make mountpoint checks namespace aware
>
> From: Ian Kent <ikent@xxxxxxxxxx>
>
> If an automount mount is clone(2)ed into a file system that is
> propagation private, when it later expires the mount subsequent
> calls to autofs ->d_automount() for in that dentry in that
> namespace will return ELOOP until the mount is manually umounted.
>
> In the same way, if an autofs mount is triggered by automount(8)
> within a container the dentry will be seen as mounted in the
> root init namespace and calls to ->d_automount() in that namespace
> will return ELOOP until the mount is umounted within the container.
>
> Also, have_submounts() can return an incorect result when a mount
> exists in a namespace other than the one being checked.
>
> Signed-off-by: Ian Kent <ikent@xxxxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
> ---
> fs/autofs4/expire.c | 4 ++--
> fs/autofs4/root.c | 14 +++++++-------
> fs/dcache.c | 2 +-
> fs/mount.h | 9 ---------
> fs/namespace.c | 1 +
> include/linux/mount.h | 9 +++++++++
> 6 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
> index 9510d8d..23b9701 100644
> --- a/fs/autofs4/expire.c
> +++ b/fs/autofs4/expire.c
> @@ -236,7 +236,7 @@ static int autofs4_tree_busy(struct vfsmount *mnt,
> * count for the autofs dentry.
> * If the fs is busy update the expiry counter.
> */
> - if (d_mountpoint(p)) {
> + if (is_local_mountpoint(p)) {
> if (autofs4_mount_busy(mnt, p)) {
> top_ino->last_used = jiffies;
> dput(p);
> @@ -280,7 +280,7 @@ static struct dentry *autofs4_check_leaves(struct vfsmount *mnt,
> while ((p = get_next_positive_dentry(p, parent))) {
> pr_debug("dentry %p %pd\n", p, p);
>
> - if (d_mountpoint(p)) {
> + if (is_local_mountpoint(p)) {
> /* Can we umount this guy */
> if (autofs4_mount_busy(mnt, p))
> continue;
> diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
> index 7ab9239..9ba487a 100644
> --- a/fs/autofs4/root.c
> +++ b/fs/autofs4/root.c
> @@ -123,7 +123,7 @@ static int autofs4_dir_open(struct inode *inode, struct file *file)
> * it.
> */
> spin_lock(&sbi->lookup_lock);
> - if (!d_mountpoint(dentry) && simple_empty(dentry)) {
> + if (!is_local_mountpoint(dentry) && simple_empty(dentry)) {
> spin_unlock(&sbi->lookup_lock);
> return -ENOENT;
> }
> @@ -370,15 +370,15 @@ static struct vfsmount *autofs4_d_automount(struct path *path)
>
> /*
> * If the dentry is a symlink it's equivalent to a directory
> - * having d_mountpoint() true, so there's no need to call back
> - * to the daemon.
> + * having is_local_mountpoint() true, so there's no need to
> + * call back to the daemon.
> */
> if (d_really_is_positive(dentry) && d_is_symlink(dentry)) {
> spin_unlock(&sbi->fs_lock);
> goto done;
> }
>
> - if (!d_mountpoint(dentry)) {
> + if (!is_local_mountpoint(dentry)) {
> /*
> * It's possible that user space hasn't removed directories
> * after umounting a rootless multi-mount, although it
> @@ -431,7 +431,7 @@ static int autofs4_d_manage(struct dentry *dentry, bool rcu_walk)
>
> /* The daemon never waits. */
> if (autofs4_oz_mode(sbi)) {
> - if (!d_mountpoint(dentry))
> + if (!is_local_mountpoint(dentry))
> return -EISDIR;
> return 0;
> }
> @@ -460,7 +460,7 @@ static int autofs4_d_manage(struct dentry *dentry, bool rcu_walk)
>
> if (ino->flags & (AUTOFS_INF_EXPIRING | AUTOFS_INF_NO_RCU))
> return 0;
> - if (d_mountpoint(dentry))
> + if (is_local_mountpoint(dentry))
> return 0;
> inode = d_inode_rcu(dentry);
> if (inode && S_ISLNK(inode->i_mode))
> @@ -487,7 +487,7 @@ static int autofs4_d_manage(struct dentry *dentry, bool rcu_walk)
> * we can avoid needless calls ->d_automount() and avoid
> * an incorrect ELOOP error return.
> */
> - if ((!d_mountpoint(dentry) && !simple_empty(dentry)) ||
> + if ((!is_local_mountpoint(dentry) && !simple_empty(dentry)) ||
> (d_really_is_positive(dentry) && d_is_symlink(dentry)))
> status = -EISDIR;
> }
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 32ceae3..9c42dc9 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1271,7 +1271,7 @@ rename_retry:
> static enum d_walk_ret check_mount(void *data, struct dentry *dentry)
> {
> int *ret = data;
> - if (d_mountpoint(dentry)) {
> + if (is_local_mountpoint(dentry)) {
> *ret = 1;
> return D_WALK_QUIT;
> }
> diff --git a/fs/mount.h b/fs/mount.h
> index 14db05d..c25e6e8 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -127,12 +127,3 @@ struct proc_mounts {
> };
>
> extern const struct seq_operations mounts_op;
> -
> -extern bool __is_local_mountpoint(struct dentry *dentry);
> -static inline bool is_local_mountpoint(struct dentry *dentry)
> -{
> - if (!d_mountpoint(dentry))
> - return false;
> -
> - return __is_local_mountpoint(dentry);
> -}
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4fb1691..9e1bc00 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -725,6 +725,7 @@ bool __is_local_mountpoint(struct dentry *dentry)
> out:
> return is_covered;
> }
> +EXPORT_SYMBOL(__is_local_mountpoint);
>
> static struct mountpoint *lookup_mountpoint(struct dentry *dentry)
> {
> diff --git a/include/linux/mount.h b/include/linux/mount.h
> index f822c3c..7419c0c 100644
> --- a/include/linux/mount.h
> +++ b/include/linux/mount.h
> @@ -15,6 +15,7 @@
> #include <linux/spinlock.h>
> #include <linux/seqlock.h>
> #include <linux/atomic.h>
> +#include <linux/dcache.h>
>
> struct super_block;
> struct vfsmount;
> @@ -95,4 +96,12 @@ extern void mark_mounts_for_expiry(struct list_head *mounts);
>
> extern dev_t name_to_dev_t(const char *name);
>
> +extern bool __is_local_mountpoint(struct dentry *dentry);
> +static inline bool is_local_mountpoint(struct dentry *dentry)
> +{
> + if (!d_mountpoint(dentry))
> + return false;
> +
> + return __is_local_mountpoint(dentry);
> +}
> #endif /* _LINUX_MOUNT_H */
Hi, Ian,
Just wanted to check on the status of this fix. Is this still the
approach you wanted to take/is there anything else you wanted to do with
this?
--
Omar
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
