libmount: (idmap) reuse tree FD, fix umount

* reuse already open mount tree; the tree could be already open due to
  regular mount, bind or remount operation

* umount (MNT_DETACH) old target if we created a clone (but the
  operation is not a bind)

Adedreses: https://github.com/util-linux/util-linux/issues/2130
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/hook_idmap.c b/libmount/src/hook_idmap.c
index a861c2c39ff9d41665d36662e36ab01b9014ba43..97808f32bb070d2cbac3eecf6ba0167cc3dde725 100644 (file)
--- a/libmount/src/hook_idmap.c
+++ b/libmount/src/hook_idmap.c
@@ -302,7 +302,7 @@ static int hook_mount_post(
        const int recursive = mnt_optlist_is_recursive(cxt->optlist);
        const char *target = mnt_fs_get_target(cxt->fs);
        int fd_tree = -1;
-       int rc;
+       int rc, is_private = 1;
 
        assert(hd);
        assert(target);
@@ -314,7 +314,19 @@ static int hook_mount_post(
         * Once a mount has been attached to the filesystem it can't be
         * idmapped anymore. So create a new detached mount.
         */
-       fd_tree = open_tree(-1, target,
+#ifdef USE_LIBMOUNT_MOUNTFD_SUPPORT
+       {
+               struct libmnt_sysapi *api = mnt_context_get_sysapi(cxt);
+
+               if (api && api->fd_tree >= 0) {
+                       fd_tree = api->fd_tree;
+                       is_private = 0;
+                       DBG(HOOK, ul_debugobj(hs, " reuse tree FD"));
+               }
+       }
+#endif
+       if (fd_tree < 0)
+               fd_tree = open_tree(-1, target,
                            OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC |
                            (recursive ? AT_RECURSIVE : 0));
        if (fd_tree < 0) {
@@ -330,20 +342,19 @@ static int hook_mount_post(
                DBG(HOOK, ul_debugobj(hs, " failed to set attributes"));
                goto done;
        }
-       /* Unmount the old, non-idmapped mount we just cloned and idmapped. */
-       rc = umount(target);
-       if (rc < 0) {
-               DBG(HOOK, ul_debugobj(hs, " failed to set umount target"));
-               goto done;
-       }
 
        /* Attach the idmapped mount. */
-       rc = move_mount(fd_tree, "", -1, target, MOVE_MOUNT_F_EMPTY_PATH);
-       if (rc)
-               DBG(HOOK, ul_debugobj(hs, " failed to set move mount"));
+       if (is_private) {
+               /* Unmount the old, non-idmapped mount we just cloned and idmapped. */
+               umount2(target, MNT_DETACH);
 
+               rc = move_mount(fd_tree, "", -1, target, MOVE_MOUNT_F_EMPTY_PATH);
+               if (rc)
+                       DBG(HOOK, ul_debugobj(hs, " failed to set move mount"));
+       }
 done:
-       close(fd_tree);
+       if (is_private)
+               close(fd_tree);
        if (rc < 0)
                return -MNT_ERR_IDMAP;
 
@@ -456,6 +467,7 @@ static int hook_prepare_options(
 done:
        /* define post-mount hook to enter the namespace */
        DBG(HOOK, ul_debugobj(hs, " wanted new user namespace"));
+       cxt->force_clone = 1; /* require OPEN_TREE_CLONE */
        rc = mnt_context_append_hook(cxt, hs,
                                MNT_STAGE_MOUNT_POST,
                                hd, hook_mount_post);

diff --git a/libmount/src/hook_mount.c b/libmount/src/hook_mount.c
index 39fc841d509bf977c71accebf385f02068bd5a73..071dad783dde8bbd881bea6daa3d883960b8552c 100644 (file)
--- a/libmount/src/hook_mount.c
+++ b/libmount/src/hook_mount.c
@@ -154,10 +154,10 @@ static int configure_superblock(struct libmnt_context *cxt,
        return 0;
 }
 
-static int open_fs_configuration_context(struct libmnt_context *cxt, const char *type)
+static int open_fs_configuration_context(struct libmnt_context *cxt,
+                                        struct libmnt_sysapi *api,
+                                        const char *type)
 {
-       int fd;
-
        DBG(HOOK, ul_debug(" new FS '%s'", type));
 
        if (!type)
@@ -165,11 +165,12 @@ static int open_fs_configuration_context(struct libmnt_context *cxt, const char
 
        DBG(HOOK, ul_debug(" fsopen(%s)", type));
 
-       fd = fsopen(type, FSOPEN_CLOEXEC);
-       set_syscall_status(cxt, "fsopen", fd >= 0);
-       if (fd < 0)
+       api->fd_fs = fsopen(type, FSOPEN_CLOEXEC);
+       set_syscall_status(cxt, "fsopen", api->fd_fs >= 0);
+       if (api->fd_fs < 0)
                return -errno;
-       return fd;
+       api->is_new_fs = 1;
+       return api->fd_fs;
 }
 
 static int open_mount_tree(struct libmnt_context *cxt, const char *path, unsigned long mflg)
@@ -197,7 +198,12 @@ static int open_mount_tree(struct libmnt_context *cxt, const char *path, unsigne
                        oflg |= AT_RECURSIVE;
        }
 
-       DBG(HOOK, ul_debug("open_tree(path=%s, flgs=0x%08lx)", path, oflg));
+       if (cxt->force_clone)
+               oflg |= OPEN_TREE_CLONE;
+
+       DBG(HOOK, ul_debug("open_tree(path=%s%s%s)", path,
+                               oflg & OPEN_TREE_CLONE ? " clone" : "",
+                               oflg & AT_RECURSIVE ? " recursive" : ""));
        fd = open_tree(AT_FDCWD, path, oflg);
        set_syscall_status(cxt, "open_tree", fd >= 0);
 
@@ -221,8 +227,8 @@ static int hook_create_mount(struct libmnt_context *cxt,
        if (api->fd_fs < 0) {
                const char *type = mnt_fs_get_fstype(cxt->fs);
 
-               api->fd_fs = open_fs_configuration_context(cxt, type);
-               if (api->fd_fs < 0) {
+               rc = open_fs_configuration_context(cxt, api, type);
+               if (rc < 0) {
                        rc = api->fd_fs;
                        goto done;
                }
@@ -452,12 +458,20 @@ static int hook_attach_target(struct libmnt_context *cxt,
        assert(api);
        assert(api->fd_tree >= 0);
 
-       if (!rc) {
-               DBG(HOOK, ul_debugobj(hs, "move_mount(to=%s)", target));
+       DBG(HOOK, ul_debugobj(hs, "move_mount(to=%s)", target));
+
+       /* umount old target if we created a clone */
+       if (cxt->force_clone
+           && !api->is_new_fs
+           && !mnt_optlist_is_bind(cxt->optlist)) {
 
-               rc = move_mount(api->fd_tree, "", AT_FDCWD, target, MOVE_MOUNT_F_EMPTY_PATH);
-               set_syscall_status(cxt, "move_mount", rc == 0);
+               DBG(HOOK, ul_debugobj(hs, "remove expired target"));
+               umount2(target, MNT_DETACH);
        }
+
+       rc = move_mount(api->fd_tree, "", AT_FDCWD, target, MOVE_MOUNT_F_EMPTY_PATH);
+       set_syscall_status(cxt, "move_mount", rc == 0);
+
        return rc == 0 ? 0 : -errno;
 }
 
@@ -515,11 +529,10 @@ static int init_sysapi(struct libmnt_context *cxt,
 
                if (mnt_context_is_fake(cxt))
                        goto fake;
-               if (cxt->helper == NULL && type && !strchr(type, ',')) {
-                       api->fd_fs = open_fs_configuration_context(cxt, type);
-                       if (api->fd_fs < 0)
-                               goto fail;
-               }
+               if (cxt->helper == NULL
+                   && type && !strchr(type, ',')
+                   && open_fs_configuration_context(cxt, api, type) < 0)
+                       goto fail;
        }
 
        return 0;
@@ -617,8 +630,8 @@ static int hook_prepare(struct libmnt_context *cxt,
        /* call move_mount() to attach target */
        if (!rc
            && cxt->helper == NULL
-           && !(flags & MS_REMOUNT)
-           && !mnt_context_propagation_only(cxt))
+           && (cxt->force_clone ||
+                       (!(flags & MS_REMOUNT) && !mnt_context_propagation_only(cxt))))
                rc = mnt_context_append_hook(cxt, hs, MNT_STAGE_MOUNT_POST, NULL,
                                        hook_attach_target);
 

diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h
index d522799c59cf56540b0b49da690aac8846d59a09..7e280a4f8b2d7e5817abb8bf9b442ed32a83688e 100644 (file)
--- a/libmount/src/mountP.h
+++ b/libmount/src/mountP.h
@@ -439,6 +439,7 @@ struct libmnt_context
        unsigned int    enabled_textdomain : 1; /* bindtextdomain() called */
        unsigned int    noautofs : 1;           /* ignore autofs mounts */
        unsigned int    has_selinux_opt : 1;    /* temporary for broken fsconfig() syscall */
+       unsigned int    force_clone : 1;        /* OPEN_TREE_CLONE */
 
        struct list_head        hooksets_datas; /* global hooksets data */
        struct list_head        hooksets_hooks; /* global hooksets data */
@@ -664,6 +665,8 @@ extern uint64_t btrfs_get_default_subvol_id(const char *path);
 struct libmnt_sysapi {
        int     fd_fs;          /* FD from fsopen() or fspick() */
        int     fd_tree;        /* FD from fsmount() or open_tree() */
+
+       unsigned int is_new_fs : 1 ;    /* fd_fs comes from fsopen() */
 };
 
 static inline struct libmnt_sysapi *mnt_context_get_sysapi(struct libmnt_context *cxt)