fs: allow changing idmappings
authorChristian Brauner <brauner@kernel.org>
Tue, 28 Jan 2025 10:33:43 +0000 (11:33 +0100)
committerChristian Brauner <brauner@kernel.org>
Wed, 12 Feb 2025 11:12:28 +0000 (12:12 +0100)
This patchset makes it possible to create a new idmapped mount from an
already idmapped mount and to clear idmappings.

// Create a first idmapped mount
struct mount_attr attr = {
        .attr_set = MOUNT_ATTR_IDMAP
        .userns_fd = fd_userns
};

fd_tree = open_tree(-EBADF, "/", OPEN_TREE_CLONE, &attr, sizeof(attr));
move_mount(fd_tree, "", -EBADF, "/mnt", MOVE_MOUNT_F_EMPTY_PATH);

// Create a second idmapped mount from the first idmapped mount
attr.attr_set = MOUNT_ATTR_IDMAP;
attr.userns_fd = fd_userns2;
fd_tree2 = open_tree(-EBADF, "/mnt", OPEN_TREE_CLONE, &attr, sizeof(attr));

// Create a second non-idmapped mount from the first idmapped mount:
memset(&attr, 0, sizeof(attr));
attr.attr_clr = MOUNT_ATTR_IDMAP;
fd_tree2 = open_tree(-EBADF, "/mnt", OPEN_TREE_CLONE, &attr, sizeof(attr));

Link: https://lore.kernel.org/r/20250128-work-mnt_idmap-update-v2-v1-5-c25feb0d2eb3@kernel.org
Reviewed-by: "Seth Forshee (DigitalOcean)" <sforshee@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>
fs/namespace.c

index a6d3f20..1d8d14c 100644 (file)
@@ -89,6 +89,7 @@ static LIST_HEAD(mnt_ns_list); /* protected by mnt_ns_tree_lock */
 
 enum mount_kattr_flags_t {
        MOUNT_KATTR_RECURSE             = (1 << 0),
+       MOUNT_KATTR_IDMAP_REPLACE       = (1 << 1),
 };
 
 struct mount_kattr {
@@ -4612,11 +4613,10 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
                return -EINVAL;
 
        /*
-        * Once a mount has been idmapped we don't allow it to change its
-        * mapping. It makes things simpler and callers can just create
-        * another bind-mount they can idmap if they want to.
+        * We only allow an mount to change it's idmapping if it has
+        * never been accessible to userspace.
         */
-       if (is_idmapped_mnt(m))
+       if (!(kattr->kflags & MOUNT_KATTR_IDMAP_REPLACE) && is_idmapped_mnt(m))
                return -EPERM;
 
        /* The underlying filesystem doesn't support idmapped mounts yet. */
@@ -4706,18 +4706,16 @@ static int mount_setattr_prepare(struct mount_kattr *kattr, struct mount *mnt)
 
 static void do_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
 {
+       struct mnt_idmap *old_idmap;
+
        if (!kattr->mnt_idmap)
                return;
 
-       /*
-        * Pairs with smp_load_acquire() in mnt_idmap().
-        *
-        * Since we only allow a mount to change the idmapping once and
-        * verified this in can_idmap_mount() we know that the mount has
-        * @nop_mnt_idmap attached to it. So there's no need to drop any
-        * references.
-        */
+       old_idmap = mnt_idmap(&mnt->mnt);
+
+       /* Pairs with smp_load_acquire() in mnt_idmap(). */
        smp_store_release(&mnt->mnt.mnt_idmap, mnt_idmap_get(kattr->mnt_idmap));
+       mnt_idmap_put(old_idmap);
 }
 
 static void mount_setattr_commit(struct mount_kattr *kattr, struct mount *mnt)
@@ -4826,13 +4824,23 @@ static int build_mount_idmapped(const struct mount_attr *attr, size_t usize,
        if (!((attr->attr_set | attr->attr_clr) & MOUNT_ATTR_IDMAP))
                return 0;
 
-       /*
-        * We currently do not support clearing an idmapped mount. If this ever
-        * is a use-case we can revisit this but for now let's keep it simple
-        * and not allow it.
-        */
-       if (attr->attr_clr & MOUNT_ATTR_IDMAP)
-               return -EINVAL;
+       if (attr->attr_clr & MOUNT_ATTR_IDMAP) {
+               /*
+                * We can only remove an idmapping if it's never been
+                * exposed to userspace.
+                */
+               if (!(kattr->kflags & MOUNT_KATTR_IDMAP_REPLACE))
+                       return -EINVAL;
+
+               /*
+                * Removal of idmappings is equivalent to setting
+                * nop_mnt_idmap.
+                */
+               if (!(attr->attr_set & MOUNT_ATTR_IDMAP)) {
+                       kattr->mnt_idmap = &nop_mnt_idmap;
+                       return 0;
+               }
+       }
 
        if (attr->userns_fd > INT_MAX)
                return -EINVAL;
@@ -4923,8 +4931,10 @@ static int build_mount_kattr(const struct mount_attr *attr, size_t usize,
 
 static void finish_mount_kattr(struct mount_kattr *kattr)
 {
-       put_user_ns(kattr->mnt_userns);
-       kattr->mnt_userns = NULL;
+       if (kattr->mnt_userns) {
+               put_user_ns(kattr->mnt_userns);
+               kattr->mnt_userns = NULL;
+       }
 
        if (kattr->mnt_idmap)
                mnt_idmap_put(kattr->mnt_idmap);
@@ -5019,6 +5029,7 @@ SYSCALL_DEFINE5(open_tree_attr, int, dfd, const char __user *, filename,
                int ret;
                struct mount_kattr kattr = {};
 
+               kattr.kflags = MOUNT_KATTR_IDMAP_REPLACE;
                if (flags & AT_RECURSIVE)
                        kattr.kflags |= MOUNT_KATTR_RECURSE;