On 21/09/2018 17:30, David Howells wrote:
From: Al Viro <v...@zeniv.linux.org.uk>

Allow a detached tree created by open_tree(..., OPEN_TREE_CLONE) to be
attached by move_mount(2).

If by the time of final fput() of OPEN_TREE_CLONE-opened file its tree is
not detached anymore, it won't be dissolved.  move_mount(2) is adjusted
to handle detached source.

That gives us equivalents of mount --bind and mount --rbind.

Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
Signed-off-by: David Howells <dhowe...@redhat.com>
---

  fs/namespace.c |   26 ++++++++++++++++++++------
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index dd38141b1723..caf5c55ef555 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1785,8 +1785,10 @@ void dissolve_on_fput(struct vfsmount *mnt)
  {
        namespace_lock();
        lock_mount_hash();
-       mntget(mnt);
-       umount_tree(real_mount(mnt), UMOUNT_CONNECTED);
+       if (!real_mount(mnt)->mnt_ns) {
+               mntget(mnt);
+               umount_tree(real_mount(mnt), UMOUNT_CONNECTED);
+       }
        unlock_mount_hash();
        namespace_unlock();
  }
@@ -2393,6 +2395,7 @@ static int do_move_mount(struct path *old_path, struct 
path *new_path)
        struct mount *old;
        struct mountpoint *mp;
        int err;
+       bool attached;
mp = lock_mount(new_path);
        err = PTR_ERR(mp);
@@ -2403,10 +2406,19 @@ static int do_move_mount(struct path *old_path, struct 
path *new_path)
        p = real_mount(new_path->mnt);
err = -EINVAL;
-       if (!check_mnt(p) || !check_mnt(old))
+       /* The mountpoint must be in our namespace. */
+       if (!check_mnt(p))
+               goto out1;
+       /* The thing moved should be either ours or completely unattached. */
+       if (old->mnt_ns && !check_mnt(old))
                goto out1;
- if (!mnt_has_parent(old))
+       attached = mnt_has_parent(old);
+       /*
+        * We need to allow open_tree(OPEN_TREE_CLONE) followed by
+        * move_mount(), but mustn't allow "/" to be moved.
+        */
+       if (old->mnt_ns && !attached)
                goto out1;
if (old->mnt.mnt_flags & MNT_LOCKED)
@@ -2421,7 +2433,7 @@ static int do_move_mount(struct path *old_path, struct 
path *new_path)
        /*
         * Don't move a mount residing in a shared parent.
         */
-       if (IS_MNT_SHARED(old->mnt_parent))
+       if (attached && IS_MNT_SHARED(old->mnt_parent))
                goto out1;
        /*
         * Don't move a mount tree containing unbindable mounts to a destination
@@ -2435,7 +2447,7 @@ static int do_move_mount(struct path *old_path, struct 
path *new_path)
                        goto out1;
err = attach_recursive_mnt(old, real_mount(new_path->mnt), mp,
-                                  &parent_path);
+                                  attached ? &parent_path : NULL);
        if (err)
                goto out1;

I think there's another small hole. It is possible to move a sub-mount from a detached tree (instead of moving the root of the tree). Then do_move_mount() calls attach_recursive_mnt() with a non-NULL parent_path.

This causes it to skip count_mounts(). So, it doesn't count the number of attached mounts, and it allows you to exceed sysctl_mount_max.

Regards
Alan

(I've tested to confirm the code lets you move a sub-mount. I didn't test whether it allows exceeding sysctl_mount_max.

# unshare -m --propagation private
# mkdir -p /tmp/mnt
# mount --bind /tmp/mnt /tmp/mnt
# open_tree_clone 3</tmp 3 sh
# cd /proc/self/fd/3
# mount --move mnt /mnt
# exit
exit
# exit
logout
#

Reply via email to