* [PATCH v2 01/10] fs: add is_idmapped_mnt() helper
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:09 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 02/10] fs: move mapping helpers Christian Brauner
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Multiple places open-code the same check to determine whether a given
mount is idmapped. Introduce a simple helper function that can be used
instead. This allows us to get rid of the fragile open-coding. We will
later change the check that is used to determine whether a given mount
is idmapped. Introducing a helper allows us to do this in a single
place instead of doing it for multiple places.
Link: https://lore.kernel.org/r/20211123114227.3124056-2-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Amir Goldstein <amir73il@gmail.com>:
- s/is_mapped_mnt/is_idmapped_mnt/g
---
fs/cachefiles/bind.c | 2 +-
fs/ecryptfs/main.c | 2 +-
fs/namespace.c | 2 +-
fs/nfsd/export.c | 2 +-
fs/overlayfs/super.c | 2 +-
fs/proc_namespace.c | 2 +-
include/linux/fs.h | 14 ++++++++++++++
7 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
index d463d89f5db8..146291be6263 100644
--- a/fs/cachefiles/bind.c
+++ b/fs/cachefiles/bind.c
@@ -117,7 +117,7 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache)
root = path.dentry;
ret = -EINVAL;
- if (mnt_user_ns(path.mnt) != &init_user_ns) {
+ if (is_idmapped_mnt(path.mnt)) {
pr_warn("File cache on idmapped mounts not supported");
goto error_unsupported;
}
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index d66bbd2df191..2dd23a82e0de 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -537,7 +537,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
goto out_free;
}
- if (mnt_user_ns(path.mnt) != &init_user_ns) {
+ if (is_idmapped_mnt(path.mnt)) {
rc = -EINVAL;
printk(KERN_ERR "Mounting on idmapped mounts currently disallowed\n");
goto out_free;
diff --git a/fs/namespace.c b/fs/namespace.c
index 659a8f39c61a..4994b816a74c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3936,7 +3936,7 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
* mapping. It makes things simpler and callers can just create
* another bind-mount they can idmap if they want to.
*/
- if (mnt_user_ns(m) != &init_user_ns)
+ if (is_idmapped_mnt(m))
return -EPERM;
/* The underlying filesystem doesn't support idmapped mounts yet. */
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 9421dae22737..668c7527b17e 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -427,7 +427,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
return -EINVAL;
}
- if (mnt_user_ns(path->mnt) != &init_user_ns) {
+ if (is_idmapped_mnt(path->mnt)) {
dprintk("exp_export: export of idmapped mounts not yet supported.\n");
return -EINVAL;
}
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 265181c110ae..7bb0a47cb615 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -873,7 +873,7 @@ static int ovl_mount_dir_noesc(const char *name, struct path *path)
pr_err("filesystem on '%s' not supported\n", name);
goto out_put;
}
- if (mnt_user_ns(path->mnt) != &init_user_ns) {
+ if (is_idmapped_mnt(path->mnt)) {
pr_err("idmapped layers are currently not supported\n");
goto out_put;
}
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 392ef5162655..49650e54d2f8 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -80,7 +80,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
seq_puts(m, fs_infop->str);
}
- if (mnt_user_ns(mnt) != &init_user_ns)
+ if (is_idmapped_mnt(mnt))
seq_puts(m, ",idmapped");
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1cb616fc1105..426cc7bcbeb8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2725,6 +2725,20 @@ static inline struct user_namespace *file_mnt_user_ns(struct file *file)
{
return mnt_user_ns(file->f_path.mnt);
}
+
+/**
+ * is_idmapped_mnt - check whether a mount is mapped
+ * @mnt: the mount to check
+ *
+ * If @mnt has an idmapping attached to it @mnt is mapped.
+ *
+ * Return: true if mount is mapped, false if not.
+ */
+static inline bool is_idmapped_mnt(const struct vfsmount *mnt)
+{
+ return mnt_user_ns(mnt) != &init_user_ns;
+}
+
extern long vfs_truncate(const struct path *, loff_t);
int do_truncate(struct user_namespace *, struct dentry *, loff_t start,
unsigned int time_attrs, struct file *filp);
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 01/10] fs: add is_idmapped_mnt() helper
2021-11-30 12:10 ` [PATCH v2 01/10] fs: add is_idmapped_mnt() helper Christian Brauner
@ 2021-12-02 17:09 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:09 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:23PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Multiple places open-code the same check to determine whether a given
> mount is idmapped. Introduce a simple helper function that can be used
> instead. This allows us to get rid of the fragile open-coding. We will
> later change the check that is used to determine whether a given mount
> is idmapped. Introducing a helper allows us to do this in a single
> place instead of doing it for multiple places.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-2-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> - Amir Goldstein <amir73il@gmail.com>:
> - s/is_mapped_mnt/is_idmapped_mnt/g
> ---
> fs/cachefiles/bind.c | 2 +-
> fs/ecryptfs/main.c | 2 +-
> fs/namespace.c | 2 +-
> fs/nfsd/export.c | 2 +-
> fs/overlayfs/super.c | 2 +-
> fs/proc_namespace.c | 2 +-
> include/linux/fs.h | 14 ++++++++++++++
> 7 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
> index d463d89f5db8..146291be6263 100644
> --- a/fs/cachefiles/bind.c
> +++ b/fs/cachefiles/bind.c
> @@ -117,7 +117,7 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache)
> root = path.dentry;
>
> ret = -EINVAL;
> - if (mnt_user_ns(path.mnt) != &init_user_ns) {
> + if (is_idmapped_mnt(path.mnt)) {
> pr_warn("File cache on idmapped mounts not supported");
> goto error_unsupported;
> }
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index d66bbd2df191..2dd23a82e0de 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -537,7 +537,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
> goto out_free;
> }
>
> - if (mnt_user_ns(path.mnt) != &init_user_ns) {
> + if (is_idmapped_mnt(path.mnt)) {
> rc = -EINVAL;
> printk(KERN_ERR "Mounting on idmapped mounts currently disallowed\n");
> goto out_free;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 659a8f39c61a..4994b816a74c 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3936,7 +3936,7 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
> * mapping. It makes things simpler and callers can just create
> * another bind-mount they can idmap if they want to.
> */
> - if (mnt_user_ns(m) != &init_user_ns)
> + if (is_idmapped_mnt(m))
> return -EPERM;
>
> /* The underlying filesystem doesn't support idmapped mounts yet. */
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 9421dae22737..668c7527b17e 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -427,7 +427,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
> return -EINVAL;
> }
>
> - if (mnt_user_ns(path->mnt) != &init_user_ns) {
> + if (is_idmapped_mnt(path->mnt)) {
> dprintk("exp_export: export of idmapped mounts not yet supported.\n");
> return -EINVAL;
> }
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 265181c110ae..7bb0a47cb615 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -873,7 +873,7 @@ static int ovl_mount_dir_noesc(const char *name, struct path *path)
> pr_err("filesystem on '%s' not supported\n", name);
> goto out_put;
> }
> - if (mnt_user_ns(path->mnt) != &init_user_ns) {
> + if (is_idmapped_mnt(path->mnt)) {
> pr_err("idmapped layers are currently not supported\n");
> goto out_put;
> }
> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> index 392ef5162655..49650e54d2f8 100644
> --- a/fs/proc_namespace.c
> +++ b/fs/proc_namespace.c
> @@ -80,7 +80,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
> seq_puts(m, fs_infop->str);
> }
>
> - if (mnt_user_ns(mnt) != &init_user_ns)
> + if (is_idmapped_mnt(mnt))
> seq_puts(m, ",idmapped");
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1cb616fc1105..426cc7bcbeb8 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2725,6 +2725,20 @@ static inline struct user_namespace *file_mnt_user_ns(struct file *file)
> {
> return mnt_user_ns(file->f_path.mnt);
> }
> +
> +/**
> + * is_idmapped_mnt - check whether a mount is mapped
> + * @mnt: the mount to check
> + *
> + * If @mnt has an idmapping attached to it @mnt is mapped.
> + *
> + * Return: true if mount is mapped, false if not.
> + */
> +static inline bool is_idmapped_mnt(const struct vfsmount *mnt)
> +{
> + return mnt_user_ns(mnt) != &init_user_ns;
> +}
> +
> extern long vfs_truncate(const struct path *, loff_t);
> int do_truncate(struct user_namespace *, struct dentry *, loff_t start,
> unsigned int time_attrs, struct file *filp);
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 02/10] fs: move mapping helpers
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
2021-11-30 12:10 ` [PATCH v2 01/10] fs: add is_idmapped_mnt() helper Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:10 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 03/10] fs: tweak fsuidgid_has_mapping() Christian Brauner
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
The low-level mapping helpers were so far crammed into fs.h. They are
out of place there. The fs.h header should just contain the higher-level
mapping helpers that interact directly with vfs objects such as struct
super_block or struct inode and not the bare mapping helpers. Similarly,
only vfs and specific fs code shall interact with low-level mapping
helpers. And so they won't be made accessible automatically through
regular {g,u}id helpers.
Link: https://lore.kernel.org/r/20211123114227.3124056-3-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Amir Goldstein <amir73il@gmail.com>:
- Rename header to mnt_idmapping.h
- Include mnt_idmapping.h header in all files where it is used to
avoid unnecessary recompliation on header change.
---
fs/ksmbd/smbacl.c | 1 +
fs/ksmbd/smbacl.h | 1 +
fs/open.c | 1 +
fs/posix_acl.c | 1 +
fs/xfs/xfs_linux.h | 1 +
include/linux/fs.h | 91 +-----------------------------
include/linux/mnt_idmapping.h | 101 ++++++++++++++++++++++++++++++++++
security/commoncap.c | 1 +
8 files changed, 108 insertions(+), 90 deletions(-)
create mode 100644 include/linux/mnt_idmapping.h
diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
index bd792db32623..ab8099e0fd7f 100644
--- a/fs/ksmbd/smbacl.c
+++ b/fs/ksmbd/smbacl.c
@@ -9,6 +9,7 @@
#include <linux/fs.h>
#include <linux/slab.h>
#include <linux/string.h>
+#include <linux/mnt_idmapping.h>
#include "smbacl.h"
#include "smb_common.h"
diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h
index 73e08cad412b..eba1ebb9e92e 100644
--- a/fs/ksmbd/smbacl.h
+++ b/fs/ksmbd/smbacl.h
@@ -11,6 +11,7 @@
#include <linux/fs.h>
#include <linux/namei.h>
#include <linux/posix_acl.h>
+#include <linux/mnt_idmapping.h>
#include "mgmt/tree_connect.h"
diff --git a/fs/open.c b/fs/open.c
index f732fb94600c..2450cc1a2f64 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -32,6 +32,7 @@
#include <linux/ima.h>
#include <linux/dnotify.h>
#include <linux/compat.h>
+#include <linux/mnt_idmapping.h>
#include "internal.h"
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 9323a854a60a..632bfdcf7cc0 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -23,6 +23,7 @@
#include <linux/export.h>
#include <linux/user_namespace.h>
#include <linux/namei.h>
+#include <linux/mnt_idmapping.h>
static struct posix_acl **acl_by_type(struct inode *inode, int type)
{
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index c174262a074e..09a8fba84ff9 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -61,6 +61,7 @@ typedef __u32 xfs_nlink_t;
#include <linux/ratelimit.h>
#include <linux/rhashtable.h>
#include <linux/xattr.h>
+#include <linux/mnt_idmapping.h>
#include <asm/page.h>
#include <asm/div64.h>
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 426cc7bcbeb8..f5d1ae0a783a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -41,6 +41,7 @@
#include <linux/stddef.h>
#include <linux/mount.h>
#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
#include <asm/byteorder.h>
#include <uapi/linux/fs.h>
@@ -1624,34 +1625,6 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
}
-/**
- * kuid_into_mnt - map a kuid down into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kuid: kuid to be mapped
- *
- * Return: @kuid mapped according to @mnt_userns.
- * If @kuid has no mapping INVALID_UID is returned.
- */
-static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
- kuid_t kuid)
-{
- return make_kuid(mnt_userns, __kuid_val(kuid));
-}
-
-/**
- * kgid_into_mnt - map a kgid down into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kgid: kgid to be mapped
- *
- * Return: @kgid mapped according to @mnt_userns.
- * If @kgid has no mapping INVALID_GID is returned.
- */
-static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
- kgid_t kgid)
-{
- return make_kgid(mnt_userns, __kgid_val(kgid));
-}
-
/**
* i_uid_into_mnt - map an inode's i_uid down into a mnt_userns
* @mnt_userns: user namespace of the mount the inode was found from
@@ -1680,68 +1653,6 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
return kgid_into_mnt(mnt_userns, inode->i_gid);
}
-/**
- * kuid_from_mnt - map a kuid up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kuid: kuid to be mapped
- *
- * Return: @kuid mapped up according to @mnt_userns.
- * If @kuid has no mapping INVALID_UID is returned.
- */
-static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
- kuid_t kuid)
-{
- return KUIDT_INIT(from_kuid(mnt_userns, kuid));
-}
-
-/**
- * kgid_from_mnt - map a kgid up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kgid: kgid to be mapped
- *
- * Return: @kgid mapped up according to @mnt_userns.
- * If @kgid has no mapping INVALID_GID is returned.
- */
-static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
- kgid_t kgid)
-{
- return KGIDT_INIT(from_kgid(mnt_userns, kgid));
-}
-
-/**
- * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- *
- * Use this helper to initialize a new vfs or filesystem object based on
- * the caller's fsuid. A common example is initializing the i_uid field of
- * a newly allocated inode triggered by a creation event such as mkdir or
- * O_CREAT. Other examples include the allocation of quotas for a specific
- * user.
- *
- * Return: the caller's current fsuid mapped up according to @mnt_userns.
- */
-static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
-{
- return kuid_from_mnt(mnt_userns, current_fsuid());
-}
-
-/**
- * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- *
- * Use this helper to initialize a new vfs or filesystem object based on
- * the caller's fsgid. A common example is initializing the i_gid field of
- * a newly allocated inode triggered by a creation event such as mkdir or
- * O_CREAT. Other examples include the allocation of quotas for a specific
- * user.
- *
- * Return: the caller's current fsgid mapped up according to @mnt_userns.
- */
-static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
-{
- return kgid_from_mnt(mnt_userns, current_fsgid());
-}
-
/**
* inode_fsuid_set - initialize inode's i_uid field with callers fsuid
* @inode: inode to initialize
diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
new file mode 100644
index 000000000000..7ff8b66b80cb
--- /dev/null
+++ b/include/linux/mnt_idmapping.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MNT_MAPPING_H
+#define _LINUX_MNT_MAPPING_H
+
+#include <linux/types.h>
+#include <linux/uidgid.h>
+
+struct user_namespace;
+extern struct user_namespace init_user_ns;
+
+/**
+ * kuid_into_mnt - map a kuid down into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kuid: kuid to be mapped
+ *
+ * Return: @kuid mapped according to @mnt_userns.
+ * If @kuid has no mapping INVALID_UID is returned.
+ */
+static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
+ kuid_t kuid)
+{
+ return make_kuid(mnt_userns, __kuid_val(kuid));
+}
+
+/**
+ * kgid_into_mnt - map a kgid down into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return: @kgid mapped according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
+static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
+ kgid_t kgid)
+{
+ return make_kgid(mnt_userns, __kgid_val(kgid));
+}
+
+/**
+ * kuid_from_mnt - map a kuid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kuid: kuid to be mapped
+ *
+ * Return: @kuid mapped up according to @mnt_userns.
+ * If @kuid has no mapping INVALID_UID is returned.
+ */
+static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
+ kuid_t kuid)
+{
+ return KUIDT_INIT(from_kuid(mnt_userns, kuid));
+}
+
+/**
+ * kgid_from_mnt - map a kgid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return: @kgid mapped up according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
+static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
+ kgid_t kgid)
+{
+ return KGIDT_INIT(from_kgid(mnt_userns, kgid));
+}
+
+/**
+ * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ *
+ * Use this helper to initialize a new vfs or filesystem object based on
+ * the caller's fsuid. A common example is initializing the i_uid field of
+ * a newly allocated inode triggered by a creation event such as mkdir or
+ * O_CREAT. Other examples include the allocation of quotas for a specific
+ * user.
+ *
+ * Return: the caller's current fsuid mapped up according to @mnt_userns.
+ */
+static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
+{
+ return kuid_from_mnt(mnt_userns, current_fsuid());
+}
+
+/**
+ * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ *
+ * Use this helper to initialize a new vfs or filesystem object based on
+ * the caller's fsgid. A common example is initializing the i_gid field of
+ * a newly allocated inode triggered by a creation event such as mkdir or
+ * O_CREAT. Other examples include the allocation of quotas for a specific
+ * user.
+ *
+ * Return: the caller's current fsgid mapped up according to @mnt_userns.
+ */
+static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
+{
+ return kgid_from_mnt(mnt_userns, current_fsgid());
+}
+
+#endif /* _LINUX_MNT_MAPPING_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index 3f810d37b71b..09479f71ee2e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -24,6 +24,7 @@
#include <linux/user_namespace.h>
#include <linux/binfmts.h>
#include <linux/personality.h>
+#include <linux/mnt_idmapping.h>
/*
* If a non-root user executes a setuid-root binary in
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 02/10] fs: move mapping helpers
2021-11-30 12:10 ` [PATCH v2 02/10] fs: move mapping helpers Christian Brauner
@ 2021-12-02 17:10 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:10 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:24PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> The low-level mapping helpers were so far crammed into fs.h. They are
> out of place there. The fs.h header should just contain the higher-level
> mapping helpers that interact directly with vfs objects such as struct
> super_block or struct inode and not the bare mapping helpers. Similarly,
> only vfs and specific fs code shall interact with low-level mapping
> helpers. And so they won't be made accessible automatically through
> regular {g,u}id helpers.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-3-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> - Amir Goldstein <amir73il@gmail.com>:
> - Rename header to mnt_idmapping.h
> - Include mnt_idmapping.h header in all files where it is used to
> avoid unnecessary recompliation on header change.
> ---
> fs/ksmbd/smbacl.c | 1 +
> fs/ksmbd/smbacl.h | 1 +
> fs/open.c | 1 +
> fs/posix_acl.c | 1 +
> fs/xfs/xfs_linux.h | 1 +
> include/linux/fs.h | 91 +-----------------------------
> include/linux/mnt_idmapping.h | 101 ++++++++++++++++++++++++++++++++++
> security/commoncap.c | 1 +
> 8 files changed, 108 insertions(+), 90 deletions(-)
> create mode 100644 include/linux/mnt_idmapping.h
>
> diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
> index bd792db32623..ab8099e0fd7f 100644
> --- a/fs/ksmbd/smbacl.c
> +++ b/fs/ksmbd/smbacl.c
> @@ -9,6 +9,7 @@
> #include <linux/fs.h>
> #include <linux/slab.h>
> #include <linux/string.h>
> +#include <linux/mnt_idmapping.h>
>
> #include "smbacl.h"
> #include "smb_common.h"
> diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h
> index 73e08cad412b..eba1ebb9e92e 100644
> --- a/fs/ksmbd/smbacl.h
> +++ b/fs/ksmbd/smbacl.h
> @@ -11,6 +11,7 @@
> #include <linux/fs.h>
> #include <linux/namei.h>
> #include <linux/posix_acl.h>
> +#include <linux/mnt_idmapping.h>
>
> #include "mgmt/tree_connect.h"
>
> diff --git a/fs/open.c b/fs/open.c
> index f732fb94600c..2450cc1a2f64 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -32,6 +32,7 @@
> #include <linux/ima.h>
> #include <linux/dnotify.h>
> #include <linux/compat.h>
> +#include <linux/mnt_idmapping.h>
>
> #include "internal.h"
>
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 9323a854a60a..632bfdcf7cc0 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -23,6 +23,7 @@
> #include <linux/export.h>
> #include <linux/user_namespace.h>
> #include <linux/namei.h>
> +#include <linux/mnt_idmapping.h>
>
> static struct posix_acl **acl_by_type(struct inode *inode, int type)
> {
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index c174262a074e..09a8fba84ff9 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -61,6 +61,7 @@ typedef __u32 xfs_nlink_t;
> #include <linux/ratelimit.h>
> #include <linux/rhashtable.h>
> #include <linux/xattr.h>
> +#include <linux/mnt_idmapping.h>
>
> #include <asm/page.h>
> #include <asm/div64.h>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 426cc7bcbeb8..f5d1ae0a783a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -41,6 +41,7 @@
> #include <linux/stddef.h>
> #include <linux/mount.h>
> #include <linux/cred.h>
> +#include <linux/mnt_idmapping.h>
>
> #include <asm/byteorder.h>
> #include <uapi/linux/fs.h>
> @@ -1624,34 +1625,6 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
> inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
> }
>
> -/**
> - * kuid_into_mnt - map a kuid down into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kuid: kuid to be mapped
> - *
> - * Return: @kuid mapped according to @mnt_userns.
> - * If @kuid has no mapping INVALID_UID is returned.
> - */
> -static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
> - kuid_t kuid)
> -{
> - return make_kuid(mnt_userns, __kuid_val(kuid));
> -}
> -
> -/**
> - * kgid_into_mnt - map a kgid down into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kgid: kgid to be mapped
> - *
> - * Return: @kgid mapped according to @mnt_userns.
> - * If @kgid has no mapping INVALID_GID is returned.
> - */
> -static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
> - kgid_t kgid)
> -{
> - return make_kgid(mnt_userns, __kgid_val(kgid));
> -}
> -
> /**
> * i_uid_into_mnt - map an inode's i_uid down into a mnt_userns
> * @mnt_userns: user namespace of the mount the inode was found from
> @@ -1680,68 +1653,6 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
> return kgid_into_mnt(mnt_userns, inode->i_gid);
> }
>
> -/**
> - * kuid_from_mnt - map a kuid up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kuid: kuid to be mapped
> - *
> - * Return: @kuid mapped up according to @mnt_userns.
> - * If @kuid has no mapping INVALID_UID is returned.
> - */
> -static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
> - kuid_t kuid)
> -{
> - return KUIDT_INIT(from_kuid(mnt_userns, kuid));
> -}
> -
> -/**
> - * kgid_from_mnt - map a kgid up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kgid: kgid to be mapped
> - *
> - * Return: @kgid mapped up according to @mnt_userns.
> - * If @kgid has no mapping INVALID_GID is returned.
> - */
> -static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
> - kgid_t kgid)
> -{
> - return KGIDT_INIT(from_kgid(mnt_userns, kgid));
> -}
> -
> -/**
> - * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - *
> - * Use this helper to initialize a new vfs or filesystem object based on
> - * the caller's fsuid. A common example is initializing the i_uid field of
> - * a newly allocated inode triggered by a creation event such as mkdir or
> - * O_CREAT. Other examples include the allocation of quotas for a specific
> - * user.
> - *
> - * Return: the caller's current fsuid mapped up according to @mnt_userns.
> - */
> -static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> -{
> - return kuid_from_mnt(mnt_userns, current_fsuid());
> -}
> -
> -/**
> - * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - *
> - * Use this helper to initialize a new vfs or filesystem object based on
> - * the caller's fsgid. A common example is initializing the i_gid field of
> - * a newly allocated inode triggered by a creation event such as mkdir or
> - * O_CREAT. Other examples include the allocation of quotas for a specific
> - * user.
> - *
> - * Return: the caller's current fsgid mapped up according to @mnt_userns.
> - */
> -static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
> -{
> - return kgid_from_mnt(mnt_userns, current_fsgid());
> -}
> -
> /**
> * inode_fsuid_set - initialize inode's i_uid field with callers fsuid
> * @inode: inode to initialize
> diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
> new file mode 100644
> index 000000000000..7ff8b66b80cb
> --- /dev/null
> +++ b/include/linux/mnt_idmapping.h
> @@ -0,0 +1,101 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_MNT_MAPPING_H
> +#define _LINUX_MNT_MAPPING_H
> +
> +#include <linux/types.h>
> +#include <linux/uidgid.h>
> +
> +struct user_namespace;
> +extern struct user_namespace init_user_ns;
> +
> +/**
> + * kuid_into_mnt - map a kuid down into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + * @kuid: kuid to be mapped
> + *
> + * Return: @kuid mapped according to @mnt_userns.
> + * If @kuid has no mapping INVALID_UID is returned.
> + */
> +static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
> + kuid_t kuid)
> +{
> + return make_kuid(mnt_userns, __kuid_val(kuid));
> +}
> +
> +/**
> + * kgid_into_mnt - map a kgid down into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + * @kgid: kgid to be mapped
> + *
> + * Return: @kgid mapped according to @mnt_userns.
> + * If @kgid has no mapping INVALID_GID is returned.
> + */
> +static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
> + kgid_t kgid)
> +{
> + return make_kgid(mnt_userns, __kgid_val(kgid));
> +}
> +
> +/**
> + * kuid_from_mnt - map a kuid up into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + * @kuid: kuid to be mapped
> + *
> + * Return: @kuid mapped up according to @mnt_userns.
> + * If @kuid has no mapping INVALID_UID is returned.
> + */
> +static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
> + kuid_t kuid)
> +{
> + return KUIDT_INIT(from_kuid(mnt_userns, kuid));
> +}
> +
> +/**
> + * kgid_from_mnt - map a kgid up into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + * @kgid: kgid to be mapped
> + *
> + * Return: @kgid mapped up according to @mnt_userns.
> + * If @kgid has no mapping INVALID_GID is returned.
> + */
> +static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
> + kgid_t kgid)
> +{
> + return KGIDT_INIT(from_kgid(mnt_userns, kgid));
> +}
> +
> +/**
> + * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + *
> + * Use this helper to initialize a new vfs or filesystem object based on
> + * the caller's fsuid. A common example is initializing the i_uid field of
> + * a newly allocated inode triggered by a creation event such as mkdir or
> + * O_CREAT. Other examples include the allocation of quotas for a specific
> + * user.
> + *
> + * Return: the caller's current fsuid mapped up according to @mnt_userns.
> + */
> +static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> +{
> + return kuid_from_mnt(mnt_userns, current_fsuid());
> +}
> +
> +/**
> + * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
> + * @mnt_userns: user namespace of the relevant mount
> + *
> + * Use this helper to initialize a new vfs or filesystem object based on
> + * the caller's fsgid. A common example is initializing the i_gid field of
> + * a newly allocated inode triggered by a creation event such as mkdir or
> + * O_CREAT. Other examples include the allocation of quotas for a specific
> + * user.
> + *
> + * Return: the caller's current fsgid mapped up according to @mnt_userns.
> + */
> +static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
> +{
> + return kgid_from_mnt(mnt_userns, current_fsgid());
> +}
> +
> +#endif /* _LINUX_MNT_MAPPING_H */
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 3f810d37b71b..09479f71ee2e 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -24,6 +24,7 @@
> #include <linux/user_namespace.h>
> #include <linux/binfmts.h>
> #include <linux/personality.h>
> +#include <linux/mnt_idmapping.h>
>
> /*
> * If a non-root user executes a setuid-root binary in
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 03/10] fs: tweak fsuidgid_has_mapping()
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
2021-11-30 12:10 ` [PATCH v2 01/10] fs: add is_idmapped_mnt() helper Christian Brauner
2021-11-30 12:10 ` [PATCH v2 02/10] fs: move mapping helpers Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:12 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 04/10] fs: account for filesystem mappings Christian Brauner
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
If the caller's fs{g,u}id aren't mapped in the mount's idmapping we can
return early and skip the check whether the mapped fs{g,u}id also have a
mapping in the filesystem's idmapping. If the fs{g,u}id aren't mapped in
the mount's idmapping they consequently can't be mapped in the
filesystem's idmapping. So there's no point in checking that.
Link: https://lore.kernel.org/r/20211123114227.3124056-4-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
include/linux/fs.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f5d1ae0a783a..28ab20ce0adc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1695,10 +1695,18 @@ static inline void inode_fsgid_set(struct inode *inode,
static inline bool fsuidgid_has_mapping(struct super_block *sb,
struct user_namespace *mnt_userns)
{
- struct user_namespace *s_user_ns = sb->s_user_ns;
+ struct user_namespace *fs_userns = sb->s_user_ns;
+ kuid_t kuid;
+ kgid_t kgid;
- return kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) &&
- kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns));
+ kuid = mapped_fsuid(mnt_userns);
+ if (!uid_valid(kuid))
+ return false;
+ kgid = mapped_fsgid(mnt_userns);
+ if (!gid_valid(kgid))
+ return false;
+ return kuid_has_mapping(fs_userns, kuid) &&
+ kgid_has_mapping(fs_userns, kgid);
}
extern struct timespec64 current_time(struct inode *inode);
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 03/10] fs: tweak fsuidgid_has_mapping()
2021-11-30 12:10 ` [PATCH v2 03/10] fs: tweak fsuidgid_has_mapping() Christian Brauner
@ 2021-12-02 17:12 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:12 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:25PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> If the caller's fs{g,u}id aren't mapped in the mount's idmapping we can
> return early and skip the check whether the mapped fs{g,u}id also have a
> mapping in the filesystem's idmapping. If the fs{g,u}id aren't mapped in
> the mount's idmapping they consequently can't be mapped in the
> filesystem's idmapping. So there's no point in checking that.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-4-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> unchanged
> ---
> include/linux/fs.h | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f5d1ae0a783a..28ab20ce0adc 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1695,10 +1695,18 @@ static inline void inode_fsgid_set(struct inode *inode,
> static inline bool fsuidgid_has_mapping(struct super_block *sb,
> struct user_namespace *mnt_userns)
> {
> - struct user_namespace *s_user_ns = sb->s_user_ns;
> + struct user_namespace *fs_userns = sb->s_user_ns;
> + kuid_t kuid;
> + kgid_t kgid;
>
> - return kuid_has_mapping(s_user_ns, mapped_fsuid(mnt_userns)) &&
> - kgid_has_mapping(s_user_ns, mapped_fsgid(mnt_userns));
> + kuid = mapped_fsuid(mnt_userns);
> + if (!uid_valid(kuid))
> + return false;
> + kgid = mapped_fsgid(mnt_userns);
> + if (!gid_valid(kgid))
> + return false;
> + return kuid_has_mapping(fs_userns, kuid) &&
> + kgid_has_mapping(fs_userns, kgid);
> }
>
> extern struct timespec64 current_time(struct inode *inode);
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 04/10] fs: account for filesystem mappings
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (2 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 03/10] fs: tweak fsuidgid_has_mapping() Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:13 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 05/10] docs: update mapping documentation Christian Brauner
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Currently we only support idmapped mounts for filesystems mounted
without an idmapping. This was a conscious decision mentioned in
multiple places (cf. e.g. [1]).
As explained at length in [3] it is perfectly fine to extend support for
idmapped mounts to filesystem's mounted with an idmapping should the
need arise. The need has been there for some time now. Various container
projects in userspace need this to run unprivileged and nested
unprivileged containers (cf. [2]).
Before we can port any filesystem that is mountable with an idmapping to
support idmapped mounts we need to first extend the mapping helpers to
account for the filesystem's idmapping. This again, is explained at
length in our documentation at [3] but I'll give an overview here again.
Currently, the low-level mapping helpers implement the remapping
algorithms described in [3] in a simplified manner. Because we could
rely on the fact that all filesystems supporting idmapped mounts are
mounted without an idmapping the translation step from or into the
filesystem idmapping could be skipped.
In order to support idmapped mounts of filesystem's mountable with an
idmapping the translation step we were able to skip before cannot be
skipped anymore. A filesystem mounted with an idmapping is very likely
to not use an identity mapping and will instead use a non-identity
mapping. So the translation step from or into the filesystem's idmapping
in the remapping algorithm cannot be skipped for such filesystems. More
details with examples can be found in [3].
This patch adds a few new and prepares some already existing low-level
mapping helpers to perform the full translation algorithm explained in
[3]. The low-level helpers can be written in a way that they only
perform the additional translation step when the filesystem is indeed
mounted with an idmapping.
If the low-level helpers detect that they are not dealing with an
idmapped mount they can simply return the relevant k{g,u}id unchanged;
no remapping needs to be performed at all. The no_mapping() helper
detects whether the shortcut can be used.
If the low-level helpers detected that they are dealing with an idmapped
mount but the underlying filesystem is mounted without an idmapping we
can rely on the previous shorcut and can continue to skip the
translation step from or into the filesystem's idmapping.
These checks guarantee that only the minimal amount of work is
performed. As before, if idmapped mounts aren't used the low-level
helpers are idempotent and no work is performed at all.
This patch adds the helpers mapped_k{g,u}id_fs() and
mapped_k{g,u}id_user(). Following patches will port all places to
replace the old k{g,u}id_into_mnt() and k{g,u}id_from_mnt() with these
two new helpers. After the conversion is done k{g,u}id_into_mnt() and
k{g,u}id_from_mnt() will be removed. This also concludes the renaming of
the mapping helpers we started in [4]. Now, all mapping helpers will
started with the "mapped_" prefix making everything nice and consistent.
The mapped_k{g,u}id_fs() helpers replace the k{g,u}id_into_mnt()
helpers. They are to be used when k{g,u}ids are to be mapped from the
vfs, e.g. from from struct inode's i_{g,u}id. Conversely, the
mapped_k{g,u}id_user() helpers replace the k{g,u}id_from_mnt() helpers.
They are to be used when k{g,u}ids are to be written to disk, e.g. when
entering from a system call to change ownership of a file.
This patch only introduces the helpers. It doesn't yet convert the
relevant places to account for filesystem mounted with an idmapping.
[1]: commit 2ca4dcc4909d ("fs/mount_setattr: tighten permission checks")
[2]: https://github.com/containers/podman/issues/10374
[3]: Documentations/filesystems/idmappings.rst
[4]: commit a65e58e791a1 ("fs: document and rename fsid helpers")
Link: https://lore.kernel.org/r/20211123114227.3124056-5-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
include/linux/fs.h | 4 +-
include/linux/mnt_idmapping.h | 193 +++++++++++++++++++++++++++++++++-
2 files changed, 191 insertions(+), 6 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 28ab20ce0adc..25de7f6ecd81 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1636,7 +1636,7 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
- return kuid_into_mnt(mnt_userns, inode->i_uid);
+ return mapped_kuid_fs(mnt_userns, &init_user_ns, inode->i_uid);
}
/**
@@ -1650,7 +1650,7 @@ static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
- return kgid_into_mnt(mnt_userns, inode->i_gid);
+ return mapped_kgid_fs(mnt_userns, &init_user_ns, inode->i_gid);
}
/**
diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
index 7ff8b66b80cb..1c75d4e0b123 100644
--- a/include/linux/mnt_idmapping.h
+++ b/include/linux/mnt_idmapping.h
@@ -6,6 +6,11 @@
#include <linux/uidgid.h>
struct user_namespace;
+/*
+ * Carries the initial idmapping of 0:0:4294967295 which is an identity
+ * mapping. This means that {g,u}id 0 is mapped to {g,u}id 0, {g,u}id 1 is
+ * mapped to {g,u}id 1, [...], {g,u}id 1000 to {g,u}id 1000, [...].
+ */
extern struct user_namespace init_user_ns;
/**
@@ -64,9 +69,189 @@ static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
return KGIDT_INIT(from_kgid(mnt_userns, kgid));
}
+/**
+ * initial_mapping - check whether this is the initial mapping
+ * @ns: idmapping to check
+ *
+ * Check whether this is the initial mapping, mapping 0 to 0, 1 to 1,
+ * [...], 1000 to 1000 [...].
+ *
+ * Return: true if this is the initial mapping, false if not.
+ */
+static inline bool initial_mapping(const struct user_namespace *ns)
+{
+ return ns == &init_user_ns;
+}
+
+/**
+ * no_mapping - check whether we can skip remapping a kuid/gid
+ * @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
+ *
+ * This function can be used to check whether a remapping between two
+ * idmappings is required.
+ * An idmapped mount is a mount that has an idmapping attached to it that
+ * is different from the filsystem's idmapping and the initial idmapping.
+ * If the initial mapping is used or the idmapping of the mount and the
+ * filesystem are identical no remapping is required.
+ *
+ * Return: true if remapping can be skipped, false if not.
+ */
+static inline bool no_mapping(const struct user_namespace *mnt_userns,
+ const struct user_namespace *fs_userns)
+{
+ return initial_mapping(mnt_userns) || mnt_userns == fs_userns;
+}
+
+/**
+ * mapped_kuid_fs - map a filesystem kuid into a mnt_userns
+ * @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
+ * @kuid : kuid to be mapped
+ *
+ * Take a @kuid and remap it from @fs_userns into @mnt_userns. Use this
+ * function when preparing a @kuid to be reported to userspace.
+ *
+ * If no_mapping() determines that this is not an idmapped mount we can
+ * simply return @kuid unchanged.
+ * If initial_mapping() tells us that the filesystem is not mounted with an
+ * idmapping we know the value of @kuid won't change when calling
+ * from_kuid() so we can simply retrieve the value via __kuid_val()
+ * directly.
+ *
+ * Return: @kuid mapped according to @mnt_userns.
+ * If @kuid has no mapping in either @mnt_userns or @fs_userns INVALID_UID is
+ * returned.
+ */
+static inline kuid_t mapped_kuid_fs(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns,
+ kuid_t kuid)
+{
+ uid_t uid;
+
+ if (no_mapping(mnt_userns, fs_userns))
+ return kuid;
+ if (initial_mapping(fs_userns))
+ uid = __kuid_val(kuid);
+ else
+ uid = from_kuid(fs_userns, kuid);
+ if (uid == (uid_t)-1)
+ return INVALID_UID;
+ return make_kuid(mnt_userns, uid);
+}
+
+/**
+ * mapped_kgid_fs - map a filesystem kgid into a mnt_userns
+ * @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
+ * @kgid : kgid to be mapped
+ *
+ * Take a @kgid and remap it from @fs_userns into @mnt_userns. Use this
+ * function when preparing a @kgid to be reported to userspace.
+ *
+ * If no_mapping() determines that this is not an idmapped mount we can
+ * simply return @kgid unchanged.
+ * If initial_mapping() tells us that the filesystem is not mounted with an
+ * idmapping we know the value of @kgid won't change when calling
+ * from_kgid() so we can simply retrieve the value via __kgid_val()
+ * directly.
+ *
+ * Return: @kgid mapped according to @mnt_userns.
+ * If @kgid has no mapping in either @mnt_userns or @fs_userns INVALID_GID is
+ * returned.
+ */
+static inline kgid_t mapped_kgid_fs(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns,
+ kgid_t kgid)
+{
+ gid_t gid;
+
+ if (no_mapping(mnt_userns, fs_userns))
+ return kgid;
+ if (initial_mapping(fs_userns))
+ gid = __kgid_val(kgid);
+ else
+ gid = from_kgid(fs_userns, kgid);
+ if (gid == (gid_t)-1)
+ return INVALID_GID;
+ return make_kgid(mnt_userns, gid);
+}
+
+/**
+ * mapped_kuid_user - map a user kuid into a mnt_userns
+ * @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
+ * @kuid : kuid to be mapped
+ *
+ * Use the idmapping of @mnt_userns to remap a @kuid into @fs_userns. Use this
+ * function when preparing a @kuid to be written to disk or inode.
+ *
+ * If no_mapping() determines that this is not an idmapped mount we can
+ * simply return @kuid unchanged.
+ * If initial_mapping() tells us that the filesystem is not mounted with an
+ * idmapping we know the value of @kuid won't change when calling
+ * make_kuid() so we can simply retrieve the value via KUIDT_INIT()
+ * directly.
+ *
+ * Return: @kuid mapped according to @mnt_userns.
+ * If @kuid has no mapping in either @mnt_userns or @fs_userns INVALID_UID is
+ * returned.
+ */
+static inline kuid_t mapped_kuid_user(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns,
+ kuid_t kuid)
+{
+ uid_t uid;
+
+ if (no_mapping(mnt_userns, fs_userns))
+ return kuid;
+ uid = from_kuid(mnt_userns, kuid);
+ if (uid == (uid_t)-1)
+ return INVALID_UID;
+ if (initial_mapping(fs_userns))
+ return KUIDT_INIT(uid);
+ return make_kuid(fs_userns, uid);
+}
+
+/**
+ * mapped_kgid_user - map a user kgid into a mnt_userns
+ * @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
+ * @kgid : kgid to be mapped
+ *
+ * Use the idmapping of @mnt_userns to remap a @kgid into @fs_userns. Use this
+ * function when preparing a @kgid to be written to disk or inode.
+ *
+ * If no_mapping() determines that this is not an idmapped mount we can
+ * simply return @kgid unchanged.
+ * If initial_mapping() tells us that the filesystem is not mounted with an
+ * idmapping we know the value of @kgid won't change when calling
+ * make_kgid() so we can simply retrieve the value via KGIDT_INIT()
+ * directly.
+ *
+ * Return: @kgid mapped according to @mnt_userns.
+ * If @kgid has no mapping in either @mnt_userns or @fs_userns INVALID_GID is
+ * returned.
+ */
+static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns,
+ kgid_t kgid)
+{
+ gid_t gid;
+
+ if (no_mapping(mnt_userns, fs_userns))
+ return kgid;
+ gid = from_kgid(mnt_userns, kgid);
+ if (gid == (gid_t)-1)
+ return INVALID_GID;
+ if (initial_mapping(fs_userns))
+ return KGIDT_INIT(gid);
+ return make_kgid(fs_userns, gid);
+}
+
/**
* mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
+ * @mnt_userns: the mount's idmapping
*
* Use this helper to initialize a new vfs or filesystem object based on
* the caller's fsuid. A common example is initializing the i_uid field of
@@ -78,12 +263,12 @@ static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
*/
static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
{
- return kuid_from_mnt(mnt_userns, current_fsuid());
+ return mapped_kuid_user(mnt_userns, &init_user_ns, current_fsuid());
}
/**
* mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
+ * @mnt_userns: the mount's idmapping
*
* Use this helper to initialize a new vfs or filesystem object based on
* the caller's fsgid. A common example is initializing the i_gid field of
@@ -95,7 +280,7 @@ static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
*/
static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
{
- return kgid_from_mnt(mnt_userns, current_fsgid());
+ return mapped_kgid_user(mnt_userns, &init_user_ns, current_fsgid());
}
#endif /* _LINUX_MNT_MAPPING_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 04/10] fs: account for filesystem mappings
2021-11-30 12:10 ` [PATCH v2 04/10] fs: account for filesystem mappings Christian Brauner
@ 2021-12-02 17:13 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:13 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:26PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Currently we only support idmapped mounts for filesystems mounted
> without an idmapping. This was a conscious decision mentioned in
> multiple places (cf. e.g. [1]).
>
> As explained at length in [3] it is perfectly fine to extend support for
> idmapped mounts to filesystem's mounted with an idmapping should the
> need arise. The need has been there for some time now. Various container
> projects in userspace need this to run unprivileged and nested
> unprivileged containers (cf. [2]).
>
> Before we can port any filesystem that is mountable with an idmapping to
> support idmapped mounts we need to first extend the mapping helpers to
> account for the filesystem's idmapping. This again, is explained at
> length in our documentation at [3] but I'll give an overview here again.
>
> Currently, the low-level mapping helpers implement the remapping
> algorithms described in [3] in a simplified manner. Because we could
> rely on the fact that all filesystems supporting idmapped mounts are
> mounted without an idmapping the translation step from or into the
> filesystem idmapping could be skipped.
>
> In order to support idmapped mounts of filesystem's mountable with an
> idmapping the translation step we were able to skip before cannot be
> skipped anymore. A filesystem mounted with an idmapping is very likely
> to not use an identity mapping and will instead use a non-identity
> mapping. So the translation step from or into the filesystem's idmapping
> in the remapping algorithm cannot be skipped for such filesystems. More
> details with examples can be found in [3].
>
> This patch adds a few new and prepares some already existing low-level
> mapping helpers to perform the full translation algorithm explained in
> [3]. The low-level helpers can be written in a way that they only
> perform the additional translation step when the filesystem is indeed
> mounted with an idmapping.
>
> If the low-level helpers detect that they are not dealing with an
> idmapped mount they can simply return the relevant k{g,u}id unchanged;
> no remapping needs to be performed at all. The no_mapping() helper
> detects whether the shortcut can be used.
>
> If the low-level helpers detected that they are dealing with an idmapped
> mount but the underlying filesystem is mounted without an idmapping we
> can rely on the previous shorcut and can continue to skip the
> translation step from or into the filesystem's idmapping.
>
> These checks guarantee that only the minimal amount of work is
> performed. As before, if idmapped mounts aren't used the low-level
> helpers are idempotent and no work is performed at all.
>
> This patch adds the helpers mapped_k{g,u}id_fs() and
> mapped_k{g,u}id_user(). Following patches will port all places to
> replace the old k{g,u}id_into_mnt() and k{g,u}id_from_mnt() with these
> two new helpers. After the conversion is done k{g,u}id_into_mnt() and
> k{g,u}id_from_mnt() will be removed. This also concludes the renaming of
> the mapping helpers we started in [4]. Now, all mapping helpers will
> started with the "mapped_" prefix making everything nice and consistent.
>
> The mapped_k{g,u}id_fs() helpers replace the k{g,u}id_into_mnt()
> helpers. They are to be used when k{g,u}ids are to be mapped from the
> vfs, e.g. from from struct inode's i_{g,u}id. Conversely, the
> mapped_k{g,u}id_user() helpers replace the k{g,u}id_from_mnt() helpers.
> They are to be used when k{g,u}ids are to be written to disk, e.g. when
> entering from a system call to change ownership of a file.
>
> This patch only introduces the helpers. It doesn't yet convert the
> relevant places to account for filesystem mounted with an idmapping.
>
> [1]: commit 2ca4dcc4909d ("fs/mount_setattr: tighten permission checks")
> [2]: https://github.com/containers/podman/issues/10374
> [3]: Documentations/filesystems/idmappings.rst
> [4]: commit a65e58e791a1 ("fs: document and rename fsid helpers")
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-5-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> unchanged
> ---
> include/linux/fs.h | 4 +-
> include/linux/mnt_idmapping.h | 193 +++++++++++++++++++++++++++++++++-
> 2 files changed, 191 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 28ab20ce0adc..25de7f6ecd81 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1636,7 +1636,7 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
> static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
> const struct inode *inode)
> {
> - return kuid_into_mnt(mnt_userns, inode->i_uid);
> + return mapped_kuid_fs(mnt_userns, &init_user_ns, inode->i_uid);
> }
>
> /**
> @@ -1650,7 +1650,7 @@ static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
> static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
> const struct inode *inode)
> {
> - return kgid_into_mnt(mnt_userns, inode->i_gid);
> + return mapped_kgid_fs(mnt_userns, &init_user_ns, inode->i_gid);
> }
>
> /**
> diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
> index 7ff8b66b80cb..1c75d4e0b123 100644
> --- a/include/linux/mnt_idmapping.h
> +++ b/include/linux/mnt_idmapping.h
> @@ -6,6 +6,11 @@
> #include <linux/uidgid.h>
>
> struct user_namespace;
> +/*
> + * Carries the initial idmapping of 0:0:4294967295 which is an identity
> + * mapping. This means that {g,u}id 0 is mapped to {g,u}id 0, {g,u}id 1 is
> + * mapped to {g,u}id 1, [...], {g,u}id 1000 to {g,u}id 1000, [...].
> + */
> extern struct user_namespace init_user_ns;
>
> /**
> @@ -64,9 +69,189 @@ static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
> return KGIDT_INIT(from_kgid(mnt_userns, kgid));
> }
>
> +/**
> + * initial_mapping - check whether this is the initial mapping
> + * @ns: idmapping to check
> + *
> + * Check whether this is the initial mapping, mapping 0 to 0, 1 to 1,
> + * [...], 1000 to 1000 [...].
> + *
> + * Return: true if this is the initial mapping, false if not.
> + */
> +static inline bool initial_mapping(const struct user_namespace *ns)
> +{
> + return ns == &init_user_ns;
> +}
> +
> +/**
> + * no_mapping - check whether we can skip remapping a kuid/gid
> + * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> + *
> + * This function can be used to check whether a remapping between two
> + * idmappings is required.
> + * An idmapped mount is a mount that has an idmapping attached to it that
> + * is different from the filsystem's idmapping and the initial idmapping.
> + * If the initial mapping is used or the idmapping of the mount and the
> + * filesystem are identical no remapping is required.
> + *
> + * Return: true if remapping can be skipped, false if not.
> + */
> +static inline bool no_mapping(const struct user_namespace *mnt_userns,
> + const struct user_namespace *fs_userns)
> +{
> + return initial_mapping(mnt_userns) || mnt_userns == fs_userns;
> +}
> +
> +/**
> + * mapped_kuid_fs - map a filesystem kuid into a mnt_userns
> + * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> + * @kuid : kuid to be mapped
> + *
> + * Take a @kuid and remap it from @fs_userns into @mnt_userns. Use this
> + * function when preparing a @kuid to be reported to userspace.
> + *
> + * If no_mapping() determines that this is not an idmapped mount we can
> + * simply return @kuid unchanged.
> + * If initial_mapping() tells us that the filesystem is not mounted with an
> + * idmapping we know the value of @kuid won't change when calling
> + * from_kuid() so we can simply retrieve the value via __kuid_val()
> + * directly.
> + *
> + * Return: @kuid mapped according to @mnt_userns.
> + * If @kuid has no mapping in either @mnt_userns or @fs_userns INVALID_UID is
> + * returned.
> + */
> +static inline kuid_t mapped_kuid_fs(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns,
> + kuid_t kuid)
> +{
> + uid_t uid;
> +
> + if (no_mapping(mnt_userns, fs_userns))
> + return kuid;
> + if (initial_mapping(fs_userns))
> + uid = __kuid_val(kuid);
> + else
> + uid = from_kuid(fs_userns, kuid);
> + if (uid == (uid_t)-1)
> + return INVALID_UID;
> + return make_kuid(mnt_userns, uid);
> +}
> +
> +/**
> + * mapped_kgid_fs - map a filesystem kgid into a mnt_userns
> + * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> + * @kgid : kgid to be mapped
> + *
> + * Take a @kgid and remap it from @fs_userns into @mnt_userns. Use this
> + * function when preparing a @kgid to be reported to userspace.
> + *
> + * If no_mapping() determines that this is not an idmapped mount we can
> + * simply return @kgid unchanged.
> + * If initial_mapping() tells us that the filesystem is not mounted with an
> + * idmapping we know the value of @kgid won't change when calling
> + * from_kgid() so we can simply retrieve the value via __kgid_val()
> + * directly.
> + *
> + * Return: @kgid mapped according to @mnt_userns.
> + * If @kgid has no mapping in either @mnt_userns or @fs_userns INVALID_GID is
> + * returned.
> + */
> +static inline kgid_t mapped_kgid_fs(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns,
> + kgid_t kgid)
> +{
> + gid_t gid;
> +
> + if (no_mapping(mnt_userns, fs_userns))
> + return kgid;
> + if (initial_mapping(fs_userns))
> + gid = __kgid_val(kgid);
> + else
> + gid = from_kgid(fs_userns, kgid);
> + if (gid == (gid_t)-1)
> + return INVALID_GID;
> + return make_kgid(mnt_userns, gid);
> +}
> +
> +/**
> + * mapped_kuid_user - map a user kuid into a mnt_userns
> + * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> + * @kuid : kuid to be mapped
> + *
> + * Use the idmapping of @mnt_userns to remap a @kuid into @fs_userns. Use this
> + * function when preparing a @kuid to be written to disk or inode.
> + *
> + * If no_mapping() determines that this is not an idmapped mount we can
> + * simply return @kuid unchanged.
> + * If initial_mapping() tells us that the filesystem is not mounted with an
> + * idmapping we know the value of @kuid won't change when calling
> + * make_kuid() so we can simply retrieve the value via KUIDT_INIT()
> + * directly.
> + *
> + * Return: @kuid mapped according to @mnt_userns.
> + * If @kuid has no mapping in either @mnt_userns or @fs_userns INVALID_UID is
> + * returned.
> + */
> +static inline kuid_t mapped_kuid_user(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns,
> + kuid_t kuid)
> +{
> + uid_t uid;
> +
> + if (no_mapping(mnt_userns, fs_userns))
> + return kuid;
> + uid = from_kuid(mnt_userns, kuid);
> + if (uid == (uid_t)-1)
> + return INVALID_UID;
> + if (initial_mapping(fs_userns))
> + return KUIDT_INIT(uid);
> + return make_kuid(fs_userns, uid);
> +}
> +
> +/**
> + * mapped_kgid_user - map a user kgid into a mnt_userns
> + * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> + * @kgid : kgid to be mapped
> + *
> + * Use the idmapping of @mnt_userns to remap a @kgid into @fs_userns. Use this
> + * function when preparing a @kgid to be written to disk or inode.
> + *
> + * If no_mapping() determines that this is not an idmapped mount we can
> + * simply return @kgid unchanged.
> + * If initial_mapping() tells us that the filesystem is not mounted with an
> + * idmapping we know the value of @kgid won't change when calling
> + * make_kgid() so we can simply retrieve the value via KGIDT_INIT()
> + * directly.
> + *
> + * Return: @kgid mapped according to @mnt_userns.
> + * If @kgid has no mapping in either @mnt_userns or @fs_userns INVALID_GID is
> + * returned.
> + */
> +static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns,
> + kgid_t kgid)
> +{
> + gid_t gid;
> +
> + if (no_mapping(mnt_userns, fs_userns))
> + return kgid;
> + gid = from_kgid(mnt_userns, kgid);
> + if (gid == (gid_t)-1)
> + return INVALID_GID;
> + if (initial_mapping(fs_userns))
> + return KGIDT_INIT(gid);
> + return make_kgid(fs_userns, gid);
> +}
> +
> /**
> * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> + * @mnt_userns: the mount's idmapping
> *
> * Use this helper to initialize a new vfs or filesystem object based on
> * the caller's fsuid. A common example is initializing the i_uid field of
> @@ -78,12 +263,12 @@ static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
> */
> static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> {
> - return kuid_from_mnt(mnt_userns, current_fsuid());
> + return mapped_kuid_user(mnt_userns, &init_user_ns, current_fsuid());
> }
>
> /**
> * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> + * @mnt_userns: the mount's idmapping
> *
> * Use this helper to initialize a new vfs or filesystem object based on
> * the caller's fsgid. A common example is initializing the i_gid field of
> @@ -95,7 +280,7 @@ static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> */
> static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
> {
> - return kgid_from_mnt(mnt_userns, current_fsgid());
> + return mapped_kgid_user(mnt_userns, &init_user_ns, current_fsgid());
> }
>
> #endif /* _LINUX_MNT_MAPPING_H */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 05/10] docs: update mapping documentation
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (3 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 04/10] fs: account for filesystem mappings Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:27 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 06/10] fs: use low-level mapping helpers Christian Brauner
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Now that we implement the full remapping algorithms described in our
documentation remove the section about shortcircuting them.
Link: https://lore.kernel.org/r/20211123114227.3124056-6-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
Documentation/filesystems/idmappings.rst | 72 ------------------------
1 file changed, 72 deletions(-)
diff --git a/Documentation/filesystems/idmappings.rst b/Documentation/filesystems/idmappings.rst
index 1229a75ec75d..7a879ec3b6bf 100644
--- a/Documentation/filesystems/idmappings.rst
+++ b/Documentation/filesystems/idmappings.rst
@@ -952,75 +952,3 @@ The raw userspace id that is put on disk is ``u1000`` so when the user takes
their home directory back to their home computer where they are assigned
``u1000`` using the initial idmapping and mount the filesystem with the initial
idmapping they will see all those files owned by ``u1000``.
-
-Shortcircuting
---------------
-
-Currently, the implementation of idmapped mounts enforces that the filesystem
-is mounted with the initial idmapping. The reason is simply that none of the
-filesystems that we targeted were mountable with a non-initial idmapping. But
-that might change soon enough. As we've seen above, thanks to the properties of
-idmappings the translation works for both filesystems mounted with the initial
-idmapping and filesystem with non-initial idmappings.
-
-Based on this current restriction to filesystem mounted with the initial
-idmapping two noticeable shortcuts have been taken:
-
-1. We always stash a reference to the initial user namespace in ``struct
- vfsmount``. Idmapped mounts are thus mounts that have a non-initial user
- namespace attached to them.
-
- In order to support idmapped mounts this needs to be changed. Instead of
- stashing the initial user namespace the user namespace the filesystem was
- mounted with must be stashed. An idmapped mount is then any mount that has
- a different user namespace attached then the filesystem was mounted with.
- This has no user-visible consequences.
-
-2. The translation algorithms in ``mapped_fs*id()`` and ``i_*id_into_mnt()``
- are simplified.
-
- Let's consider ``mapped_fs*id()`` first. This function translates the
- caller's kernel id into a kernel id in the filesystem's idmapping via
- a mount's idmapping. The full algorithm is::
-
- mapped_fsuid(kid):
- /* Map the kernel id up into a userspace id in the mount's idmapping. */
- from_kuid(mount-idmapping, kid) = uid
-
- /* Map the userspace id down into a kernel id in the filesystem's idmapping. */
- make_kuid(filesystem-idmapping, uid) = kuid
-
- We know that the filesystem is always mounted with the initial idmapping as
- we enforce this in ``mount_setattr()``. So this can be shortened to::
-
- mapped_fsuid(kid):
- /* Map the kernel id up into a userspace id in the mount's idmapping. */
- from_kuid(mount-idmapping, kid) = uid
-
- /* Map the userspace id down into a kernel id in the filesystem's idmapping. */
- KUIDT_INIT(uid) = kuid
-
- Similarly, for ``i_*id_into_mnt()`` which translated the filesystem's kernel
- id into a mount's kernel id::
-
- i_uid_into_mnt(kid):
- /* Map the kernel id up into a userspace id in the filesystem's idmapping. */
- from_kuid(filesystem-idmapping, kid) = uid
-
- /* Map the userspace id down into a kernel id in the mounts's idmapping. */
- make_kuid(mount-idmapping, uid) = kuid
-
- Again, we know that the filesystem is always mounted with the initial
- idmapping as we enforce this in ``mount_setattr()``. So this can be
- shortened to::
-
- i_uid_into_mnt(kid):
- /* Map the kernel id up into a userspace id in the filesystem's idmapping. */
- __kuid_val(kid) = uid
-
- /* Map the userspace id down into a kernel id in the mounts's idmapping. */
- make_kuid(mount-idmapping, uid) = kuid
-
-Handling filesystems mounted with non-initial idmappings requires that the
-translation functions be converted to their full form. They can still be
-shortcircuited on non-idmapped mounts. This has no user-visible consequences.
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 05/10] docs: update mapping documentation
2021-11-30 12:10 ` [PATCH v2 05/10] docs: update mapping documentation Christian Brauner
@ 2021-12-02 17:27 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:27 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:27PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Now that we implement the full remapping algorithms described in our
> documentation remove the section about shortcircuting them.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-6-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> unchanged
> ---
> Documentation/filesystems/idmappings.rst | 72 ------------------------
> 1 file changed, 72 deletions(-)
>
> diff --git a/Documentation/filesystems/idmappings.rst b/Documentation/filesystems/idmappings.rst
> index 1229a75ec75d..7a879ec3b6bf 100644
> --- a/Documentation/filesystems/idmappings.rst
> +++ b/Documentation/filesystems/idmappings.rst
> @@ -952,75 +952,3 @@ The raw userspace id that is put on disk is ``u1000`` so when the user takes
> their home directory back to their home computer where they are assigned
> ``u1000`` using the initial idmapping and mount the filesystem with the initial
> idmapping they will see all those files owned by ``u1000``.
> -
> -Shortcircuting
> ---------------
> -
> -Currently, the implementation of idmapped mounts enforces that the filesystem
> -is mounted with the initial idmapping. The reason is simply that none of the
> -filesystems that we targeted were mountable with a non-initial idmapping. But
> -that might change soon enough. As we've seen above, thanks to the properties of
> -idmappings the translation works for both filesystems mounted with the initial
> -idmapping and filesystem with non-initial idmappings.
> -
> -Based on this current restriction to filesystem mounted with the initial
> -idmapping two noticeable shortcuts have been taken:
> -
> -1. We always stash a reference to the initial user namespace in ``struct
> - vfsmount``. Idmapped mounts are thus mounts that have a non-initial user
> - namespace attached to them.
> -
> - In order to support idmapped mounts this needs to be changed. Instead of
> - stashing the initial user namespace the user namespace the filesystem was
> - mounted with must be stashed. An idmapped mount is then any mount that has
> - a different user namespace attached then the filesystem was mounted with.
> - This has no user-visible consequences.
> -
> -2. The translation algorithms in ``mapped_fs*id()`` and ``i_*id_into_mnt()``
> - are simplified.
> -
> - Let's consider ``mapped_fs*id()`` first. This function translates the
> - caller's kernel id into a kernel id in the filesystem's idmapping via
> - a mount's idmapping. The full algorithm is::
> -
> - mapped_fsuid(kid):
> - /* Map the kernel id up into a userspace id in the mount's idmapping. */
> - from_kuid(mount-idmapping, kid) = uid
> -
> - /* Map the userspace id down into a kernel id in the filesystem's idmapping. */
> - make_kuid(filesystem-idmapping, uid) = kuid
> -
> - We know that the filesystem is always mounted with the initial idmapping as
> - we enforce this in ``mount_setattr()``. So this can be shortened to::
> -
> - mapped_fsuid(kid):
> - /* Map the kernel id up into a userspace id in the mount's idmapping. */
> - from_kuid(mount-idmapping, kid) = uid
> -
> - /* Map the userspace id down into a kernel id in the filesystem's idmapping. */
> - KUIDT_INIT(uid) = kuid
> -
> - Similarly, for ``i_*id_into_mnt()`` which translated the filesystem's kernel
> - id into a mount's kernel id::
> -
> - i_uid_into_mnt(kid):
> - /* Map the kernel id up into a userspace id in the filesystem's idmapping. */
> - from_kuid(filesystem-idmapping, kid) = uid
> -
> - /* Map the userspace id down into a kernel id in the mounts's idmapping. */
> - make_kuid(mount-idmapping, uid) = kuid
> -
> - Again, we know that the filesystem is always mounted with the initial
> - idmapping as we enforce this in ``mount_setattr()``. So this can be
> - shortened to::
> -
> - i_uid_into_mnt(kid):
> - /* Map the kernel id up into a userspace id in the filesystem's idmapping. */
> - __kuid_val(kid) = uid
> -
> - /* Map the userspace id down into a kernel id in the mounts's idmapping. */
> - make_kuid(mount-idmapping, uid) = kuid
> -
> -Handling filesystems mounted with non-initial idmappings requires that the
> -translation functions be converted to their full form. They can still be
> -shortcircuited on non-idmapped mounts. This has no user-visible consequences.
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 06/10] fs: use low-level mapping helpers
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (4 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 05/10] docs: update mapping documentation Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:34 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 07/10] fs: remove unused " Christian Brauner
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
In a few places the vfs needs to interact with bare k{g,u}ids directly
instead of struct inode. These are just a few. In previous patches we
introduced low-level mapping helpers that are able to support
filesystems mounted an idmapping. This patch simply converts the places
to use these new helpers.
Link: https://lore.kernel.org/r/20211123114227.3124056-7-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
fs/ksmbd/smbacl.c | 18 ++----------------
fs/ksmbd/smbacl.h | 4 ++--
fs/open.c | 4 ++--
fs/posix_acl.c | 16 ++++++++++------
security/commoncap.c | 13 ++++++++-----
5 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
index ab8099e0fd7f..6ecf55ea1fed 100644
--- a/fs/ksmbd/smbacl.c
+++ b/fs/ksmbd/smbacl.c
@@ -275,14 +275,7 @@ static int sid_to_id(struct user_namespace *user_ns,
uid_t id;
id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
- /*
- * Translate raw sid into kuid in the server's user
- * namespace.
- */
- uid = make_kuid(&init_user_ns, id);
-
- /* If this is an idmapped mount, apply the idmapping. */
- uid = kuid_from_mnt(user_ns, uid);
+ uid = mapped_kuid_user(user_ns, &init_user_ns, KUIDT_INIT(id));
if (uid_valid(uid)) {
fattr->cf_uid = uid;
rc = 0;
@@ -292,14 +285,7 @@ static int sid_to_id(struct user_namespace *user_ns,
gid_t id;
id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
- /*
- * Translate raw sid into kgid in the server's user
- * namespace.
- */
- gid = make_kgid(&init_user_ns, id);
-
- /* If this is an idmapped mount, apply the idmapping. */
- gid = kgid_from_mnt(user_ns, gid);
+ gid = mapped_kgid_user(user_ns, &init_user_ns, KGIDT_INIT(id));
if (gid_valid(gid)) {
fattr->cf_gid = gid;
rc = 0;
diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h
index eba1ebb9e92e..811af3309429 100644
--- a/fs/ksmbd/smbacl.h
+++ b/fs/ksmbd/smbacl.h
@@ -217,7 +217,7 @@ static inline uid_t posix_acl_uid_translate(struct user_namespace *mnt_userns,
kuid_t kuid;
/* If this is an idmapped mount, apply the idmapping. */
- kuid = kuid_into_mnt(mnt_userns, pace->e_uid);
+ kuid = mapped_kuid_fs(mnt_userns, &init_user_ns, pace->e_uid);
/* Translate the kuid into a userspace id ksmbd would see. */
return from_kuid(&init_user_ns, kuid);
@@ -229,7 +229,7 @@ static inline gid_t posix_acl_gid_translate(struct user_namespace *mnt_userns,
kgid_t kgid;
/* If this is an idmapped mount, apply the idmapping. */
- kgid = kgid_into_mnt(mnt_userns, pace->e_gid);
+ kgid = mapped_kgid_fs(mnt_userns, &init_user_ns, pace->e_gid);
/* Translate the kgid into a userspace id ksmbd would see. */
return from_kgid(&init_user_ns, kgid);
diff --git a/fs/open.c b/fs/open.c
index 2450cc1a2f64..40a00e71865b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -653,8 +653,8 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
gid = make_kgid(current_user_ns(), group);
mnt_userns = mnt_user_ns(path->mnt);
- uid = kuid_from_mnt(mnt_userns, uid);
- gid = kgid_from_mnt(mnt_userns, gid);
+ uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
+ gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
retry_deleg:
newattrs.ia_valid = ATTR_CTIME;
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 632bfdcf7cc0..4b5fb9a9b90f 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -375,7 +375,9 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
goto check_perm;
break;
case ACL_USER:
- uid = kuid_into_mnt(mnt_userns, pa->e_uid);
+ uid = mapped_kuid_fs(mnt_userns,
+ &init_user_ns,
+ pa->e_uid);
if (uid_eq(uid, current_fsuid()))
goto mask;
break;
@@ -388,7 +390,9 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
}
break;
case ACL_GROUP:
- gid = kgid_into_mnt(mnt_userns, pa->e_gid);
+ gid = mapped_kgid_fs(mnt_userns,
+ &init_user_ns,
+ pa->e_gid);
if (in_group_p(gid)) {
found = 1;
if ((pa->e_perm & want) == want)
@@ -735,17 +739,17 @@ static void posix_acl_fix_xattr_userns(
case ACL_USER:
uid = make_kuid(from, le32_to_cpu(entry->e_id));
if (from_user)
- uid = kuid_from_mnt(mnt_userns, uid);
+ uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
else
- uid = kuid_into_mnt(mnt_userns, uid);
+ uid = mapped_kuid_fs(mnt_userns, &init_user_ns, uid);
entry->e_id = cpu_to_le32(from_kuid(to, uid));
break;
case ACL_GROUP:
gid = make_kgid(from, le32_to_cpu(entry->e_id));
if (from_user)
- gid = kgid_from_mnt(mnt_userns, gid);
+ gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
else
- gid = kgid_into_mnt(mnt_userns, gid);
+ gid = mapped_kgid_fs(mnt_userns, &init_user_ns, gid);
entry->e_id = cpu_to_le32(from_kgid(to, gid));
break;
default:
diff --git a/security/commoncap.c b/security/commoncap.c
index 09479f71ee2e..d288a62e2999 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -419,7 +419,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
kroot = make_kuid(fs_ns, root);
/* If this is an idmapped mount shift the kuid. */
- kroot = kuid_into_mnt(mnt_userns, kroot);
+ kroot = mapped_kuid_fs(mnt_userns, &init_user_ns, kroot);
/* If the root kuid maps to a valid uid in current ns, then return
* this as a nscap. */
@@ -489,6 +489,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
* @size: size of @ivalue
* @task_ns: user namespace of the caller
* @mnt_userns: user namespace of the mount the inode was found from
+ * @fs_userns: user namespace of the filesystem
*
* If the inode has been found through an idmapped mount the user namespace of
* the vfsmount must be passed through @mnt_userns. This function will then
@@ -498,7 +499,8 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
*/
static kuid_t rootid_from_xattr(const void *value, size_t size,
struct user_namespace *task_ns,
- struct user_namespace *mnt_userns)
+ struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns)
{
const struct vfs_ns_cap_data *nscap = value;
kuid_t rootkid;
@@ -508,7 +510,7 @@ static kuid_t rootid_from_xattr(const void *value, size_t size,
rootid = le32_to_cpu(nscap->rootid);
rootkid = make_kuid(task_ns, rootid);
- return kuid_from_mnt(mnt_userns, rootkid);
+ return mapped_kuid_user(mnt_userns, fs_userns, rootkid);
}
static bool validheader(size_t size, const struct vfs_cap_data *cap)
@@ -559,7 +561,8 @@ int cap_convert_nscap(struct user_namespace *mnt_userns, struct dentry *dentry,
/* user is privileged, just write the v2 */
return size;
- rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns);
+ rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns,
+ &init_user_ns);
if (!uid_valid(rootid))
return -EINVAL;
@@ -700,7 +703,7 @@ int get_vfs_caps_from_disk(struct user_namespace *mnt_userns,
/* Limit the caps to the mounter of the filesystem
* or the more limited uid specified in the xattr.
*/
- rootkuid = kuid_into_mnt(mnt_userns, rootkuid);
+ rootkuid = mapped_kuid_fs(mnt_userns, &init_user_ns, rootkuid);
if (!rootid_owns_currentns(rootkuid))
return -ENODATA;
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 06/10] fs: use low-level mapping helpers
2021-11-30 12:10 ` [PATCH v2 06/10] fs: use low-level mapping helpers Christian Brauner
@ 2021-12-02 17:34 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:34 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:28PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> In a few places the vfs needs to interact with bare k{g,u}ids directly
> instead of struct inode. These are just a few. In previous patches we
> introduced low-level mapping helpers that are able to support
> filesystems mounted an idmapping. This patch simply converts the places
> to use these new helpers.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-7-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> unchanged
> ---
> fs/ksmbd/smbacl.c | 18 ++----------------
> fs/ksmbd/smbacl.h | 4 ++--
> fs/open.c | 4 ++--
> fs/posix_acl.c | 16 ++++++++++------
> security/commoncap.c | 13 ++++++++-----
> 5 files changed, 24 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
> index ab8099e0fd7f..6ecf55ea1fed 100644
> --- a/fs/ksmbd/smbacl.c
> +++ b/fs/ksmbd/smbacl.c
> @@ -275,14 +275,7 @@ static int sid_to_id(struct user_namespace *user_ns,
> uid_t id;
>
> id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
> - /*
> - * Translate raw sid into kuid in the server's user
> - * namespace.
> - */
> - uid = make_kuid(&init_user_ns, id);
> -
> - /* If this is an idmapped mount, apply the idmapping. */
> - uid = kuid_from_mnt(user_ns, uid);
> + uid = mapped_kuid_user(user_ns, &init_user_ns, KUIDT_INIT(id));
> if (uid_valid(uid)) {
> fattr->cf_uid = uid;
> rc = 0;
> @@ -292,14 +285,7 @@ static int sid_to_id(struct user_namespace *user_ns,
> gid_t id;
>
> id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
> - /*
> - * Translate raw sid into kgid in the server's user
> - * namespace.
> - */
> - gid = make_kgid(&init_user_ns, id);
> -
> - /* If this is an idmapped mount, apply the idmapping. */
> - gid = kgid_from_mnt(user_ns, gid);
> + gid = mapped_kgid_user(user_ns, &init_user_ns, KGIDT_INIT(id));
> if (gid_valid(gid)) {
> fattr->cf_gid = gid;
> rc = 0;
> diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h
> index eba1ebb9e92e..811af3309429 100644
> --- a/fs/ksmbd/smbacl.h
> +++ b/fs/ksmbd/smbacl.h
> @@ -217,7 +217,7 @@ static inline uid_t posix_acl_uid_translate(struct user_namespace *mnt_userns,
> kuid_t kuid;
>
> /* If this is an idmapped mount, apply the idmapping. */
> - kuid = kuid_into_mnt(mnt_userns, pace->e_uid);
> + kuid = mapped_kuid_fs(mnt_userns, &init_user_ns, pace->e_uid);
>
> /* Translate the kuid into a userspace id ksmbd would see. */
> return from_kuid(&init_user_ns, kuid);
> @@ -229,7 +229,7 @@ static inline gid_t posix_acl_gid_translate(struct user_namespace *mnt_userns,
> kgid_t kgid;
>
> /* If this is an idmapped mount, apply the idmapping. */
> - kgid = kgid_into_mnt(mnt_userns, pace->e_gid);
> + kgid = mapped_kgid_fs(mnt_userns, &init_user_ns, pace->e_gid);
>
> /* Translate the kgid into a userspace id ksmbd would see. */
> return from_kgid(&init_user_ns, kgid);
> diff --git a/fs/open.c b/fs/open.c
> index 2450cc1a2f64..40a00e71865b 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -653,8 +653,8 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
> gid = make_kgid(current_user_ns(), group);
>
> mnt_userns = mnt_user_ns(path->mnt);
> - uid = kuid_from_mnt(mnt_userns, uid);
> - gid = kgid_from_mnt(mnt_userns, gid);
> + uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
> + gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
>
> retry_deleg:
> newattrs.ia_valid = ATTR_CTIME;
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 632bfdcf7cc0..4b5fb9a9b90f 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -375,7 +375,9 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
> goto check_perm;
> break;
> case ACL_USER:
> - uid = kuid_into_mnt(mnt_userns, pa->e_uid);
> + uid = mapped_kuid_fs(mnt_userns,
> + &init_user_ns,
> + pa->e_uid);
> if (uid_eq(uid, current_fsuid()))
> goto mask;
> break;
> @@ -388,7 +390,9 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
> }
> break;
> case ACL_GROUP:
> - gid = kgid_into_mnt(mnt_userns, pa->e_gid);
> + gid = mapped_kgid_fs(mnt_userns,
> + &init_user_ns,
> + pa->e_gid);
> if (in_group_p(gid)) {
> found = 1;
> if ((pa->e_perm & want) == want)
> @@ -735,17 +739,17 @@ static void posix_acl_fix_xattr_userns(
> case ACL_USER:
> uid = make_kuid(from, le32_to_cpu(entry->e_id));
> if (from_user)
> - uid = kuid_from_mnt(mnt_userns, uid);
> + uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
> else
> - uid = kuid_into_mnt(mnt_userns, uid);
> + uid = mapped_kuid_fs(mnt_userns, &init_user_ns, uid);
> entry->e_id = cpu_to_le32(from_kuid(to, uid));
> break;
> case ACL_GROUP:
> gid = make_kgid(from, le32_to_cpu(entry->e_id));
> if (from_user)
> - gid = kgid_from_mnt(mnt_userns, gid);
> + gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
> else
> - gid = kgid_into_mnt(mnt_userns, gid);
> + gid = mapped_kgid_fs(mnt_userns, &init_user_ns, gid);
> entry->e_id = cpu_to_le32(from_kgid(to, gid));
> break;
> default:
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 09479f71ee2e..d288a62e2999 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -419,7 +419,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
> kroot = make_kuid(fs_ns, root);
>
> /* If this is an idmapped mount shift the kuid. */
> - kroot = kuid_into_mnt(mnt_userns, kroot);
> + kroot = mapped_kuid_fs(mnt_userns, &init_user_ns, kroot);
>
> /* If the root kuid maps to a valid uid in current ns, then return
> * this as a nscap. */
> @@ -489,6 +489,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
> * @size: size of @ivalue
> * @task_ns: user namespace of the caller
> * @mnt_userns: user namespace of the mount the inode was found from
> + * @fs_userns: user namespace of the filesystem
> *
> * If the inode has been found through an idmapped mount the user namespace of
> * the vfsmount must be passed through @mnt_userns. This function will then
> @@ -498,7 +499,8 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
> */
> static kuid_t rootid_from_xattr(const void *value, size_t size,
> struct user_namespace *task_ns,
> - struct user_namespace *mnt_userns)
> + struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns)
> {
> const struct vfs_ns_cap_data *nscap = value;
> kuid_t rootkid;
> @@ -508,7 +510,7 @@ static kuid_t rootid_from_xattr(const void *value, size_t size,
> rootid = le32_to_cpu(nscap->rootid);
>
> rootkid = make_kuid(task_ns, rootid);
> - return kuid_from_mnt(mnt_userns, rootkid);
> + return mapped_kuid_user(mnt_userns, fs_userns, rootkid);
> }
>
> static bool validheader(size_t size, const struct vfs_cap_data *cap)
> @@ -559,7 +561,8 @@ int cap_convert_nscap(struct user_namespace *mnt_userns, struct dentry *dentry,
> /* user is privileged, just write the v2 */
> return size;
>
> - rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns);
> + rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns,
> + &init_user_ns);
> if (!uid_valid(rootid))
> return -EINVAL;
>
> @@ -700,7 +703,7 @@ int get_vfs_caps_from_disk(struct user_namespace *mnt_userns,
> /* Limit the caps to the mounter of the filesystem
> * or the more limited uid specified in the xattr.
> */
> - rootkuid = kuid_into_mnt(mnt_userns, rootkuid);
> + rootkuid = mapped_kuid_fs(mnt_userns, &init_user_ns, rootkuid);
> if (!rootid_owns_currentns(rootkuid))
> return -ENODATA;
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 07/10] fs: remove unused low-level mapping helpers
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (5 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 06/10] fs: use low-level mapping helpers Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:39 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 08/10] fs: port higher-level " Christian Brauner
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Now that we ported all places to use the new low-level mapping helpers
that are able to support filesystems mounted with an idmapping we can
remove the old low-level mapping helpers. With the removal of these old
helpers we also conclude the renaming of the mapping helpers we started
in [1].
[1]: commit a65e58e791a1 ("fs: document and rename fsid helpers")
Link: https://lore.kernel.org/r/20211123114227.3124056-8-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
include/linux/mnt_idmapping.h | 56 -----------------------------------
1 file changed, 56 deletions(-)
diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
index 1c75d4e0b123..c4b604a0c256 100644
--- a/include/linux/mnt_idmapping.h
+++ b/include/linux/mnt_idmapping.h
@@ -13,62 +13,6 @@ struct user_namespace;
*/
extern struct user_namespace init_user_ns;
-/**
- * kuid_into_mnt - map a kuid down into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kuid: kuid to be mapped
- *
- * Return: @kuid mapped according to @mnt_userns.
- * If @kuid has no mapping INVALID_UID is returned.
- */
-static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
- kuid_t kuid)
-{
- return make_kuid(mnt_userns, __kuid_val(kuid));
-}
-
-/**
- * kgid_into_mnt - map a kgid down into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kgid: kgid to be mapped
- *
- * Return: @kgid mapped according to @mnt_userns.
- * If @kgid has no mapping INVALID_GID is returned.
- */
-static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
- kgid_t kgid)
-{
- return make_kgid(mnt_userns, __kgid_val(kgid));
-}
-
-/**
- * kuid_from_mnt - map a kuid up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kuid: kuid to be mapped
- *
- * Return: @kuid mapped up according to @mnt_userns.
- * If @kuid has no mapping INVALID_UID is returned.
- */
-static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
- kuid_t kuid)
-{
- return KUIDT_INIT(from_kuid(mnt_userns, kuid));
-}
-
-/**
- * kgid_from_mnt - map a kgid up into a mnt_userns
- * @mnt_userns: user namespace of the relevant mount
- * @kgid: kgid to be mapped
- *
- * Return: @kgid mapped up according to @mnt_userns.
- * If @kgid has no mapping INVALID_GID is returned.
- */
-static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
- kgid_t kgid)
-{
- return KGIDT_INIT(from_kgid(mnt_userns, kgid));
-}
-
/**
* initial_mapping - check whether this is the initial mapping
* @ns: idmapping to check
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 07/10] fs: remove unused low-level mapping helpers
2021-11-30 12:10 ` [PATCH v2 07/10] fs: remove unused " Christian Brauner
@ 2021-12-02 17:39 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:39 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:29PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Now that we ported all places to use the new low-level mapping helpers
> that are able to support filesystems mounted with an idmapping we can
> remove the old low-level mapping helpers. With the removal of these old
> helpers we also conclude the renaming of the mapping helpers we started
> in [1].
>
> [1]: commit a65e58e791a1 ("fs: document and rename fsid helpers")
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-8-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> unchanged
> ---
> include/linux/mnt_idmapping.h | 56 -----------------------------------
> 1 file changed, 56 deletions(-)
>
> diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
> index 1c75d4e0b123..c4b604a0c256 100644
> --- a/include/linux/mnt_idmapping.h
> +++ b/include/linux/mnt_idmapping.h
> @@ -13,62 +13,6 @@ struct user_namespace;
> */
> extern struct user_namespace init_user_ns;
>
> -/**
> - * kuid_into_mnt - map a kuid down into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kuid: kuid to be mapped
> - *
> - * Return: @kuid mapped according to @mnt_userns.
> - * If @kuid has no mapping INVALID_UID is returned.
> - */
> -static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
> - kuid_t kuid)
> -{
> - return make_kuid(mnt_userns, __kuid_val(kuid));
> -}
> -
> -/**
> - * kgid_into_mnt - map a kgid down into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kgid: kgid to be mapped
> - *
> - * Return: @kgid mapped according to @mnt_userns.
> - * If @kgid has no mapping INVALID_GID is returned.
> - */
> -static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
> - kgid_t kgid)
> -{
> - return make_kgid(mnt_userns, __kgid_val(kgid));
> -}
> -
> -/**
> - * kuid_from_mnt - map a kuid up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kuid: kuid to be mapped
> - *
> - * Return: @kuid mapped up according to @mnt_userns.
> - * If @kuid has no mapping INVALID_UID is returned.
> - */
> -static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
> - kuid_t kuid)
> -{
> - return KUIDT_INIT(from_kuid(mnt_userns, kuid));
> -}
> -
> -/**
> - * kgid_from_mnt - map a kgid up into a mnt_userns
> - * @mnt_userns: user namespace of the relevant mount
> - * @kgid: kgid to be mapped
> - *
> - * Return: @kgid mapped up according to @mnt_userns.
> - * If @kgid has no mapping INVALID_GID is returned.
> - */
> -static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
> - kgid_t kgid)
> -{
> - return KGIDT_INIT(from_kgid(mnt_userns, kgid));
> -}
> -
> /**
> * initial_mapping - check whether this is the initial mapping
> * @ns: idmapping to check
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 08/10] fs: port higher-level mapping helpers
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (6 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 07/10] fs: remove unused " Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:40 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 09/10] fs: add i_user_ns() helper Christian Brauner
2021-11-30 12:10 ` [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems Christian Brauner
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Enable the mapped_fs{g,u}id() helpers to support filesystems mounted
with an idmapping. Apart from core mapping helpers that use
mapped_fs{g,u}id() to initialize struct inode's i_{g,u}id fields xfs is
the only place that uses these low-level helpers directly.
The patch only extends the helpers to be able to take the filesystem
idmapping into account. Since we don't actually yet pass the
filesystem's idmapping in no functional changes happen. This will happen
in a final patch.
Link: https://lore.kernel.org/r/20211123114227.3124056-9-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Amir Goldstein <amir73il@gmail.com>:
- Avoid using local variable and simply pass down initial idmapping
directly.
---
fs/xfs/xfs_inode.c | 8 ++++----
fs/xfs/xfs_symlink.c | 4 ++--
include/linux/fs.h | 8 ++++----
include/linux/mnt_idmapping.h | 12 ++++++++----
4 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 64b9bf334806..5ca689459bed 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -988,8 +988,8 @@ xfs_create(
/*
* Make sure that we have allocated dquot(s) on disk.
*/
- error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
- mapped_fsgid(mnt_userns), prid,
+ error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
+ mapped_fsgid(mnt_userns, &init_user_ns), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
@@ -1142,8 +1142,8 @@ xfs_create_tmpfile(
/*
* Make sure that we have allocated dquot(s) on disk.
*/
- error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
- mapped_fsgid(mnt_userns), prid,
+ error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
+ mapped_fsgid(mnt_userns, &init_user_ns), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index fc2c6a404647..a31d2e5d0321 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -184,8 +184,8 @@ xfs_symlink(
/*
* Make sure that we have allocated dquot(s) on disk.
*/
- error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
- mapped_fsgid(mnt_userns), prid,
+ error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
+ mapped_fsgid(mnt_userns, &init_user_ns), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 25de7f6ecd81..7c0499b63a02 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1664,7 +1664,7 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
static inline void inode_fsuid_set(struct inode *inode,
struct user_namespace *mnt_userns)
{
- inode->i_uid = mapped_fsuid(mnt_userns);
+ inode->i_uid = mapped_fsuid(mnt_userns, &init_user_ns);
}
/**
@@ -1678,7 +1678,7 @@ static inline void inode_fsuid_set(struct inode *inode,
static inline void inode_fsgid_set(struct inode *inode,
struct user_namespace *mnt_userns)
{
- inode->i_gid = mapped_fsgid(mnt_userns);
+ inode->i_gid = mapped_fsgid(mnt_userns, &init_user_ns);
}
/**
@@ -1699,10 +1699,10 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
kuid_t kuid;
kgid_t kgid;
- kuid = mapped_fsuid(mnt_userns);
+ kuid = mapped_fsuid(mnt_userns, &init_user_ns);
if (!uid_valid(kuid))
return false;
- kgid = mapped_fsgid(mnt_userns);
+ kgid = mapped_fsgid(mnt_userns, &init_user_ns);
if (!gid_valid(kgid))
return false;
return kuid_has_mapping(fs_userns, kuid) &&
diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
index c4b604a0c256..afd9bde6ba0d 100644
--- a/include/linux/mnt_idmapping.h
+++ b/include/linux/mnt_idmapping.h
@@ -196,6 +196,7 @@ static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
/**
* mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
* @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
*
* Use this helper to initialize a new vfs or filesystem object based on
* the caller's fsuid. A common example is initializing the i_uid field of
@@ -205,14 +206,16 @@ static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
*
* Return: the caller's current fsuid mapped up according to @mnt_userns.
*/
-static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
+static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns)
{
- return mapped_kuid_user(mnt_userns, &init_user_ns, current_fsuid());
+ return mapped_kuid_user(mnt_userns, fs_userns, current_fsuid());
}
/**
* mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
* @mnt_userns: the mount's idmapping
+ * @fs_userns: the filesystem's idmapping
*
* Use this helper to initialize a new vfs or filesystem object based on
* the caller's fsgid. A common example is initializing the i_gid field of
@@ -222,9 +225,10 @@ static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
*
* Return: the caller's current fsgid mapped up according to @mnt_userns.
*/
-static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
+static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns,
+ struct user_namespace *fs_userns)
{
- return mapped_kgid_user(mnt_userns, &init_user_ns, current_fsgid());
+ return mapped_kgid_user(mnt_userns, fs_userns, current_fsgid());
}
#endif /* _LINUX_MNT_MAPPING_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 08/10] fs: port higher-level mapping helpers
2021-11-30 12:10 ` [PATCH v2 08/10] fs: port higher-level " Christian Brauner
@ 2021-12-02 17:40 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:40 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:30PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Enable the mapped_fs{g,u}id() helpers to support filesystems mounted
> with an idmapping. Apart from core mapping helpers that use
> mapped_fs{g,u}id() to initialize struct inode's i_{g,u}id fields xfs is
> the only place that uses these low-level helpers directly.
>
> The patch only extends the helpers to be able to take the filesystem
> idmapping into account. Since we don't actually yet pass the
> filesystem's idmapping in no functional changes happen. This will happen
> in a final patch.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-9-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> ---
> /* v2 */
> - Amir Goldstein <amir73il@gmail.com>:
> - Avoid using local variable and simply pass down initial idmapping
> directly.
> ---
> fs/xfs/xfs_inode.c | 8 ++++----
> fs/xfs/xfs_symlink.c | 4 ++--
> include/linux/fs.h | 8 ++++----
> include/linux/mnt_idmapping.h | 12 ++++++++----
> 4 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 64b9bf334806..5ca689459bed 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -988,8 +988,8 @@ xfs_create(
> /*
> * Make sure that we have allocated dquot(s) on disk.
> */
> - error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
> - mapped_fsgid(mnt_userns), prid,
> + error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
> + mapped_fsgid(mnt_userns, &init_user_ns), prid,
> XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
> &udqp, &gdqp, &pdqp);
> if (error)
> @@ -1142,8 +1142,8 @@ xfs_create_tmpfile(
> /*
> * Make sure that we have allocated dquot(s) on disk.
> */
> - error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
> - mapped_fsgid(mnt_userns), prid,
> + error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
> + mapped_fsgid(mnt_userns, &init_user_ns), prid,
> XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
> &udqp, &gdqp, &pdqp);
> if (error)
> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> index fc2c6a404647..a31d2e5d0321 100644
> --- a/fs/xfs/xfs_symlink.c
> +++ b/fs/xfs/xfs_symlink.c
> @@ -184,8 +184,8 @@ xfs_symlink(
> /*
> * Make sure that we have allocated dquot(s) on disk.
> */
> - error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns),
> - mapped_fsgid(mnt_userns), prid,
> + error = xfs_qm_vop_dqalloc(dp, mapped_fsuid(mnt_userns, &init_user_ns),
> + mapped_fsgid(mnt_userns, &init_user_ns), prid,
> XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
> &udqp, &gdqp, &pdqp);
> if (error)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 25de7f6ecd81..7c0499b63a02 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1664,7 +1664,7 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
> static inline void inode_fsuid_set(struct inode *inode,
> struct user_namespace *mnt_userns)
> {
> - inode->i_uid = mapped_fsuid(mnt_userns);
> + inode->i_uid = mapped_fsuid(mnt_userns, &init_user_ns);
> }
>
> /**
> @@ -1678,7 +1678,7 @@ static inline void inode_fsuid_set(struct inode *inode,
> static inline void inode_fsgid_set(struct inode *inode,
> struct user_namespace *mnt_userns)
> {
> - inode->i_gid = mapped_fsgid(mnt_userns);
> + inode->i_gid = mapped_fsgid(mnt_userns, &init_user_ns);
> }
>
> /**
> @@ -1699,10 +1699,10 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> kuid_t kuid;
> kgid_t kgid;
>
> - kuid = mapped_fsuid(mnt_userns);
> + kuid = mapped_fsuid(mnt_userns, &init_user_ns);
> if (!uid_valid(kuid))
> return false;
> - kgid = mapped_fsgid(mnt_userns);
> + kgid = mapped_fsgid(mnt_userns, &init_user_ns);
> if (!gid_valid(kgid))
> return false;
> return kuid_has_mapping(fs_userns, kuid) &&
> diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
> index c4b604a0c256..afd9bde6ba0d 100644
> --- a/include/linux/mnt_idmapping.h
> +++ b/include/linux/mnt_idmapping.h
> @@ -196,6 +196,7 @@ static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
> /**
> * mapped_fsuid - return caller's fsuid mapped up into a mnt_userns
> * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> *
> * Use this helper to initialize a new vfs or filesystem object based on
> * the caller's fsuid. A common example is initializing the i_uid field of
> @@ -205,14 +206,16 @@ static inline kgid_t mapped_kgid_user(struct user_namespace *mnt_userns,
> *
> * Return: the caller's current fsuid mapped up according to @mnt_userns.
> */
> -static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> +static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns)
> {
> - return mapped_kuid_user(mnt_userns, &init_user_ns, current_fsuid());
> + return mapped_kuid_user(mnt_userns, fs_userns, current_fsuid());
> }
>
> /**
> * mapped_fsgid - return caller's fsgid mapped up into a mnt_userns
> * @mnt_userns: the mount's idmapping
> + * @fs_userns: the filesystem's idmapping
> *
> * Use this helper to initialize a new vfs or filesystem object based on
> * the caller's fsgid. A common example is initializing the i_gid field of
> @@ -222,9 +225,10 @@ static inline kuid_t mapped_fsuid(struct user_namespace *mnt_userns)
> *
> * Return: the caller's current fsgid mapped up according to @mnt_userns.
> */
> -static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns)
> +static inline kgid_t mapped_fsgid(struct user_namespace *mnt_userns,
> + struct user_namespace *fs_userns)
> {
> - return mapped_kgid_user(mnt_userns, &init_user_ns, current_fsgid());
> + return mapped_kgid_user(mnt_userns, fs_userns, current_fsgid());
> }
>
> #endif /* _LINUX_MNT_MAPPING_H */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 09/10] fs: add i_user_ns() helper
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (7 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 08/10] fs: port higher-level " Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:40 ` Seth Forshee
2021-11-30 12:10 ` [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems Christian Brauner
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Since we'll be passing the filesystem's idmapping in even more places in
the following patches and we do already dereference struct inode to get
to the filesystem's idmapping multiple times add a tiny helper.
Link: https://lore.kernel.org/r/20211123114227.3124056-10-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
---
include/linux/fs.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7c0499b63a02..c7f72b78ab7e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1600,6 +1600,11 @@ struct super_block {
struct list_head s_inodes_wb; /* writeback inodes */
} __randomize_layout;
+static inline struct user_namespace *i_user_ns(const struct inode *inode)
+{
+ return inode->i_sb->s_user_ns;
+}
+
/* Helper functions so that in most cases filesystems will
* not need to deal directly with kuid_t and kgid_t and can
* instead deal with the raw numeric values that are stored
@@ -1607,22 +1612,22 @@ struct super_block {
*/
static inline uid_t i_uid_read(const struct inode *inode)
{
- return from_kuid(inode->i_sb->s_user_ns, inode->i_uid);
+ return from_kuid(i_user_ns(inode), inode->i_uid);
}
static inline gid_t i_gid_read(const struct inode *inode)
{
- return from_kgid(inode->i_sb->s_user_ns, inode->i_gid);
+ return from_kgid(i_user_ns(inode), inode->i_gid);
}
static inline void i_uid_write(struct inode *inode, uid_t uid)
{
- inode->i_uid = make_kuid(inode->i_sb->s_user_ns, uid);
+ inode->i_uid = make_kuid(i_user_ns(inode), uid);
}
static inline void i_gid_write(struct inode *inode, gid_t gid)
{
- inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
+ inode->i_gid = make_kgid(i_user_ns(inode), gid);
}
/**
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 09/10] fs: add i_user_ns() helper
2021-11-30 12:10 ` [PATCH v2 09/10] fs: add i_user_ns() helper Christian Brauner
@ 2021-12-02 17:40 ` Seth Forshee
0 siblings, 0 replies; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:40 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:31PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> Since we'll be passing the filesystem's idmapping in even more places in
> the following patches and we do already dereference struct inode to get
> to the filesystem's idmapping multiple times add a tiny helper.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-10-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
> /* v2 */
> unchanged
> ---
> include/linux/fs.h | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 7c0499b63a02..c7f72b78ab7e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1600,6 +1600,11 @@ struct super_block {
> struct list_head s_inodes_wb; /* writeback inodes */
> } __randomize_layout;
>
> +static inline struct user_namespace *i_user_ns(const struct inode *inode)
> +{
> + return inode->i_sb->s_user_ns;
> +}
> +
> /* Helper functions so that in most cases filesystems will
> * not need to deal directly with kuid_t and kgid_t and can
> * instead deal with the raw numeric values that are stored
> @@ -1607,22 +1612,22 @@ struct super_block {
> */
> static inline uid_t i_uid_read(const struct inode *inode)
> {
> - return from_kuid(inode->i_sb->s_user_ns, inode->i_uid);
> + return from_kuid(i_user_ns(inode), inode->i_uid);
> }
>
> static inline gid_t i_gid_read(const struct inode *inode)
> {
> - return from_kgid(inode->i_sb->s_user_ns, inode->i_gid);
> + return from_kgid(i_user_ns(inode), inode->i_gid);
> }
>
> static inline void i_uid_write(struct inode *inode, uid_t uid)
> {
> - inode->i_uid = make_kuid(inode->i_sb->s_user_ns, uid);
> + inode->i_uid = make_kuid(i_user_ns(inode), uid);
> }
>
> static inline void i_gid_write(struct inode *inode, gid_t gid)
> {
> - inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
> + inode->i_gid = make_kgid(i_user_ns(inode), gid);
> }
>
> /**
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems
2021-11-30 12:10 [PATCH v2 00/10] Extend and tweak mapping support Christian Brauner
` (8 preceding siblings ...)
2021-11-30 12:10 ` [PATCH v2 09/10] fs: add i_user_ns() helper Christian Brauner
@ 2021-11-30 12:10 ` Christian Brauner
2021-12-02 17:50 ` Seth Forshee
9 siblings, 1 reply; 22+ messages in thread
From: Christian Brauner @ 2021-11-30 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Seth Forshee, Amir Goldstein, Al Viro, linux-fsdevel, Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
In previous patches we added new and modified existing helpers to handle
idmapped mounts of filesystems mounted with an idmapping. In this final
patch we convert all relevant places in the vfs to actually pass the
filesystem's idmapping into these helpers.
With this the vfs is in shape to handle idmapped mounts of filesystems
mounted with an idmapping. Note that this is just the generic
infrastructure. Actually adding support for idmapped mounts to a
filesystem mountable with an idmapping is follow-up work.
In this patch we extend the definition of an idmapped mount from a mount
that that has the initial idmapping attached to it to a mount that has
an idmapping attached to it which is not the same as the idmapping the
filesystem was mounted with.
As before we do not allow the initial idmapping to be attached to a
mount. In addition this patch prevents that the idmapping the filesystem
was mounted with can be attached to a mount created based on this
filesystem.
This has multiple reasons and advantages. First, attaching the initial
idmapping or the filesystem's idmapping doesn't make much sense as in
both cases the values of the i_{g,u}id and other places where k{g,u}ids
are used do not change. Second, a user that really wants to do this for
whatever reason can just create a separate dedicated identical idmapping
to attach to the mount. Third, we can continue to use the initial
idmapping as an indicator that a mount is not idmapped allowing us to
continue to keep passing the initial idmapping into the mapping helpers
to tell them that something isn't an idmapped mount even if the
filesystem is mounted with an idmapping.
Link: https://lore.kernel.org/r/20211123114227.3124056-11-brauner@kernel.org (v1)
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Amir Goldstein <amir73il@gmail.com>:
- Continue passing down the initial idmapping to xfs. Since xfs cannot
and is not planned to support idmapped superblocks don't mislead
readers in the code by passing down the filesystem idmapping. It
will be the initial idmapping always anyway.
- Include mnt_idmapping.h header into fs/namespace.c
---
fs/namespace.c | 37 +++++++++++++++++++++++++------------
fs/open.c | 7 ++++---
fs/posix_acl.c | 8 ++++----
include/linux/fs.h | 17 +++++++++--------
security/commoncap.c | 9 ++++-----
5 files changed, 46 insertions(+), 32 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 4994b816a74c..ccefede4ba1b 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -31,6 +31,7 @@
#include <uapi/linux/mount.h>
#include <linux/fs_context.h>
#include <linux/shmem_fs.h>
+#include <linux/mnt_idmapping.h>
#include "pnode.h"
#include "internal.h"
@@ -561,7 +562,7 @@ static void free_vfsmnt(struct mount *mnt)
struct user_namespace *mnt_userns;
mnt_userns = mnt_user_ns(&mnt->mnt);
- if (mnt_userns != &init_user_ns)
+ if (!initial_mapping(mnt_userns))
put_user_ns(mnt_userns);
kfree_const(mnt->mnt_devname);
#ifdef CONFIG_SMP
@@ -965,6 +966,7 @@ static struct mount *skip_mnt_tree(struct mount *p)
struct vfsmount *vfs_create_mount(struct fs_context *fc)
{
struct mount *mnt;
+ struct user_namespace *fs_userns;
if (!fc->root)
return ERR_PTR(-EINVAL);
@@ -982,6 +984,10 @@ struct vfsmount *vfs_create_mount(struct fs_context *fc)
mnt->mnt_mountpoint = mnt->mnt.mnt_root;
mnt->mnt_parent = mnt;
+ fs_userns = mnt->mnt.mnt_sb->s_user_ns;
+ if (!initial_mapping(fs_userns))
+ mnt->mnt.mnt_userns = get_user_ns(fs_userns);
+
lock_mount_hash();
list_add_tail(&mnt->mnt_instance, &mnt->mnt.mnt_sb->s_mounts);
unlock_mount_hash();
@@ -1072,7 +1078,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
atomic_inc(&sb->s_active);
mnt->mnt.mnt_userns = mnt_user_ns(&old->mnt);
- if (mnt->mnt.mnt_userns != &init_user_ns)
+ if (!initial_mapping(mnt->mnt.mnt_userns))
mnt->mnt.mnt_userns = get_user_ns(mnt->mnt.mnt_userns);
mnt->mnt.mnt_sb = sb;
mnt->mnt.mnt_root = dget(root);
@@ -3927,10 +3933,18 @@ static unsigned int recalc_flags(struct mount_kattr *kattr, struct mount *mnt)
static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
{
struct vfsmount *m = &mnt->mnt;
+ struct user_namespace *fs_userns = m->mnt_sb->s_user_ns;
if (!kattr->mnt_userns)
return 0;
+ /*
+ * Creating an idmapped mount with the filesystem wide idmapping
+ * doesn't make sense so block that. We don't allow mushy semantics.
+ */
+ if (kattr->mnt_userns == fs_userns)
+ 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
@@ -3943,12 +3957,8 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
if (!(m->mnt_sb->s_type->fs_flags & FS_ALLOW_IDMAP))
return -EINVAL;
- /* Don't yet support filesystem mountable in user namespaces. */
- if (m->mnt_sb->s_user_ns != &init_user_ns)
- return -EINVAL;
-
/* We're not controlling the superblock. */
- if (!capable(CAP_SYS_ADMIN))
+ if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
return -EPERM;
/* Mount has already been visible in the filesystem hierarchy. */
@@ -4133,13 +4143,16 @@ static int build_mount_idmapped(const struct mount_attr *attr, size_t usize,
}
/*
- * The init_user_ns is used to indicate that a vfsmount is not idmapped.
- * This is simpler than just having to treat NULL as unmapped. Users
- * wanting to idmap a mount to init_user_ns can just use a namespace
- * with an identity mapping.
+ * The initial idmapping cannot be used to create an idmapped
+ * mount. Attaching the initial idmapping doesn't make much sense
+ * as it is an identity mapping. A user can just create a dedicated
+ * identity mapping to achieve the same result. We also use the
+ * initial idmapping as an indicator of a mount that is not
+ * idmapped. It can simply be passed into helpers that are aware of
+ * idmapped mounts as a convenient shortcut.
*/
mnt_userns = container_of(ns, struct user_namespace, ns);
- if (mnt_userns == &init_user_ns) {
+ if (initial_mapping(mnt_userns)) {
err = -EPERM;
goto out_fput;
}
diff --git a/fs/open.c b/fs/open.c
index 40a00e71865b..9ff2f621b760 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -641,7 +641,7 @@ SYSCALL_DEFINE2(chmod, const char __user *, filename, umode_t, mode)
int chown_common(const struct path *path, uid_t user, gid_t group)
{
- struct user_namespace *mnt_userns;
+ struct user_namespace *mnt_userns, *fs_userns;
struct inode *inode = path->dentry->d_inode;
struct inode *delegated_inode = NULL;
int error;
@@ -653,8 +653,9 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
gid = make_kgid(current_user_ns(), group);
mnt_userns = mnt_user_ns(path->mnt);
- uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
- gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
+ fs_userns = i_user_ns(inode);
+ uid = mapped_kuid_user(mnt_userns, fs_userns, uid);
+ gid = mapped_kgid_user(mnt_userns, fs_userns, gid);
retry_deleg:
newattrs.ia_valid = ATTR_CTIME;
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 4b5fb9a9b90f..80acb6885cf9 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -376,8 +376,8 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
break;
case ACL_USER:
uid = mapped_kuid_fs(mnt_userns,
- &init_user_ns,
- pa->e_uid);
+ i_user_ns(inode),
+ pa->e_uid);
if (uid_eq(uid, current_fsuid()))
goto mask;
break;
@@ -391,8 +391,8 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
break;
case ACL_GROUP:
gid = mapped_kgid_fs(mnt_userns,
- &init_user_ns,
- pa->e_gid);
+ i_user_ns(inode),
+ pa->e_gid);
if (in_group_p(gid)) {
found = 1;
if ((pa->e_perm & want) == want)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c7f72b78ab7e..9df6903634e8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1641,7 +1641,7 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
- return mapped_kuid_fs(mnt_userns, &init_user_ns, inode->i_uid);
+ return mapped_kuid_fs(mnt_userns, i_user_ns(inode), inode->i_uid);
}
/**
@@ -1655,7 +1655,7 @@ static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
const struct inode *inode)
{
- return mapped_kgid_fs(mnt_userns, &init_user_ns, inode->i_gid);
+ return mapped_kgid_fs(mnt_userns, i_user_ns(inode), inode->i_gid);
}
/**
@@ -1669,7 +1669,7 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
static inline void inode_fsuid_set(struct inode *inode,
struct user_namespace *mnt_userns)
{
- inode->i_uid = mapped_fsuid(mnt_userns, &init_user_ns);
+ inode->i_uid = mapped_fsuid(mnt_userns, i_user_ns(inode));
}
/**
@@ -1683,7 +1683,7 @@ static inline void inode_fsuid_set(struct inode *inode,
static inline void inode_fsgid_set(struct inode *inode,
struct user_namespace *mnt_userns)
{
- inode->i_gid = mapped_fsgid(mnt_userns, &init_user_ns);
+ inode->i_gid = mapped_fsgid(mnt_userns, i_user_ns(inode));
}
/**
@@ -1704,10 +1704,10 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
kuid_t kuid;
kgid_t kgid;
- kuid = mapped_fsuid(mnt_userns, &init_user_ns);
+ kuid = mapped_fsuid(mnt_userns, fs_userns);
if (!uid_valid(kuid))
return false;
- kgid = mapped_fsgid(mnt_userns, &init_user_ns);
+ kgid = mapped_fsgid(mnt_userns, fs_userns);
if (!gid_valid(kgid))
return false;
return kuid_has_mapping(fs_userns, kuid) &&
@@ -2654,13 +2654,14 @@ static inline struct user_namespace *file_mnt_user_ns(struct file *file)
* is_idmapped_mnt - check whether a mount is mapped
* @mnt: the mount to check
*
- * If @mnt has an idmapping attached to it @mnt is mapped.
+ * If @mnt has an idmapping attached different from the
+ * filesystem's idmapping then @mnt is mapped.
*
* Return: true if mount is mapped, false if not.
*/
static inline bool is_idmapped_mnt(const struct vfsmount *mnt)
{
- return mnt_user_ns(mnt) != &init_user_ns;
+ return mnt_user_ns(mnt) != mnt->mnt_sb->s_user_ns;
}
extern long vfs_truncate(const struct path *, loff_t);
diff --git a/security/commoncap.c b/security/commoncap.c
index d288a62e2999..5fc8986c3c77 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -419,7 +419,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
kroot = make_kuid(fs_ns, root);
/* If this is an idmapped mount shift the kuid. */
- kroot = mapped_kuid_fs(mnt_userns, &init_user_ns, kroot);
+ kroot = mapped_kuid_fs(mnt_userns, fs_ns, kroot);
/* If the root kuid maps to a valid uid in current ns, then return
* this as a nscap. */
@@ -556,13 +556,12 @@ int cap_convert_nscap(struct user_namespace *mnt_userns, struct dentry *dentry,
return -EINVAL;
if (!capable_wrt_inode_uidgid(mnt_userns, inode, CAP_SETFCAP))
return -EPERM;
- if (size == XATTR_CAPS_SZ_2 && (mnt_userns == &init_user_ns))
+ if (size == XATTR_CAPS_SZ_2 && (mnt_userns == fs_ns))
if (ns_capable(inode->i_sb->s_user_ns, CAP_SETFCAP))
/* user is privileged, just write the v2 */
return size;
- rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns,
- &init_user_ns);
+ rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns, fs_ns);
if (!uid_valid(rootid))
return -EINVAL;
@@ -703,7 +702,7 @@ int get_vfs_caps_from_disk(struct user_namespace *mnt_userns,
/* Limit the caps to the mounter of the filesystem
* or the more limited uid specified in the xattr.
*/
- rootkuid = mapped_kuid_fs(mnt_userns, &init_user_ns, rootkuid);
+ rootkuid = mapped_kuid_fs(mnt_userns, fs_ns, rootkuid);
if (!rootid_owns_currentns(rootkuid))
return -ENODATA;
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems
2021-11-30 12:10 ` [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems Christian Brauner
@ 2021-12-02 17:50 ` Seth Forshee
2021-12-02 19:20 ` Christian Brauner
0 siblings, 1 reply; 22+ messages in thread
From: Seth Forshee @ 2021-12-02 17:50 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Amir Goldstein, Al Viro, linux-fsdevel,
Christian Brauner
On Tue, Nov 30, 2021 at 01:10:32PM +0100, Christian Brauner wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> In previous patches we added new and modified existing helpers to handle
> idmapped mounts of filesystems mounted with an idmapping. In this final
> patch we convert all relevant places in the vfs to actually pass the
> filesystem's idmapping into these helpers.
>
> With this the vfs is in shape to handle idmapped mounts of filesystems
> mounted with an idmapping. Note that this is just the generic
> infrastructure. Actually adding support for idmapped mounts to a
> filesystem mountable with an idmapping is follow-up work.
>
> In this patch we extend the definition of an idmapped mount from a mount
> that that has the initial idmapping attached to it to a mount that has
> an idmapping attached to it which is not the same as the idmapping the
> filesystem was mounted with.
>
> As before we do not allow the initial idmapping to be attached to a
> mount. In addition this patch prevents that the idmapping the filesystem
> was mounted with can be attached to a mount created based on this
> filesystem.
>
> This has multiple reasons and advantages. First, attaching the initial
> idmapping or the filesystem's idmapping doesn't make much sense as in
> both cases the values of the i_{g,u}id and other places where k{g,u}ids
> are used do not change. Second, a user that really wants to do this for
> whatever reason can just create a separate dedicated identical idmapping
> to attach to the mount. Third, we can continue to use the initial
> idmapping as an indicator that a mount is not idmapped allowing us to
> continue to keep passing the initial idmapping into the mapping helpers
> to tell them that something isn't an idmapped mount even if the
> filesystem is mounted with an idmapping.
>
> Link: https://lore.kernel.org/r/20211123114227.3124056-11-brauner@kernel.org (v1)
> Cc: Seth Forshee <sforshee@digitalocean.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> - Amir Goldstein <amir73il@gmail.com>:
> - Continue passing down the initial idmapping to xfs. Since xfs cannot
> and is not planned to support idmapped superblocks don't mislead
> readers in the code by passing down the filesystem idmapping. It
> will be the initial idmapping always anyway.
> - Include mnt_idmapping.h header into fs/namespace.c
> ---
> fs/namespace.c | 37 +++++++++++++++++++++++++------------
> fs/open.c | 7 ++++---
> fs/posix_acl.c | 8 ++++----
> include/linux/fs.h | 17 +++++++++--------
> security/commoncap.c | 9 ++++-----
> 5 files changed, 46 insertions(+), 32 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4994b816a74c..ccefede4ba1b 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -31,6 +31,7 @@
> #include <uapi/linux/mount.h>
> #include <linux/fs_context.h>
> #include <linux/shmem_fs.h>
> +#include <linux/mnt_idmapping.h>
>
> #include "pnode.h"
> #include "internal.h"
> @@ -561,7 +562,7 @@ static void free_vfsmnt(struct mount *mnt)
> struct user_namespace *mnt_userns;
>
> mnt_userns = mnt_user_ns(&mnt->mnt);
> - if (mnt_userns != &init_user_ns)
> + if (!initial_mapping(mnt_userns))
> put_user_ns(mnt_userns);
> kfree_const(mnt->mnt_devname);
> #ifdef CONFIG_SMP
> @@ -965,6 +966,7 @@ static struct mount *skip_mnt_tree(struct mount *p)
> struct vfsmount *vfs_create_mount(struct fs_context *fc)
> {
> struct mount *mnt;
> + struct user_namespace *fs_userns;
>
> if (!fc->root)
> return ERR_PTR(-EINVAL);
> @@ -982,6 +984,10 @@ struct vfsmount *vfs_create_mount(struct fs_context *fc)
> mnt->mnt_mountpoint = mnt->mnt.mnt_root;
> mnt->mnt_parent = mnt;
>
> + fs_userns = mnt->mnt.mnt_sb->s_user_ns;
> + if (!initial_mapping(fs_userns))
> + mnt->mnt.mnt_userns = get_user_ns(fs_userns);
> +
Won't this get be leaked if mnt_userns is overwritten by
do_idmap_mount()?
> lock_mount_hash();
> list_add_tail(&mnt->mnt_instance, &mnt->mnt.mnt_sb->s_mounts);
> unlock_mount_hash();
> @@ -1072,7 +1078,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
>
> atomic_inc(&sb->s_active);
> mnt->mnt.mnt_userns = mnt_user_ns(&old->mnt);
> - if (mnt->mnt.mnt_userns != &init_user_ns)
> + if (!initial_mapping(mnt->mnt.mnt_userns))
> mnt->mnt.mnt_userns = get_user_ns(mnt->mnt.mnt_userns);
> mnt->mnt.mnt_sb = sb;
> mnt->mnt.mnt_root = dget(root);
> @@ -3927,10 +3933,18 @@ static unsigned int recalc_flags(struct mount_kattr *kattr, struct mount *mnt)
> static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
> {
> struct vfsmount *m = &mnt->mnt;
> + struct user_namespace *fs_userns = m->mnt_sb->s_user_ns;
>
> if (!kattr->mnt_userns)
> return 0;
>
> + /*
> + * Creating an idmapped mount with the filesystem wide idmapping
> + * doesn't make sense so block that. We don't allow mushy semantics.
> + */
> + if (kattr->mnt_userns == fs_userns)
> + 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
> @@ -3943,12 +3957,8 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
> if (!(m->mnt_sb->s_type->fs_flags & FS_ALLOW_IDMAP))
> return -EINVAL;
>
> - /* Don't yet support filesystem mountable in user namespaces. */
> - if (m->mnt_sb->s_user_ns != &init_user_ns)
> - return -EINVAL;
> -
> /* We're not controlling the superblock. */
> - if (!capable(CAP_SYS_ADMIN))
> + if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
> return -EPERM;
>
> /* Mount has already been visible in the filesystem hierarchy. */
> @@ -4133,13 +4143,16 @@ static int build_mount_idmapped(const struct mount_attr *attr, size_t usize,
> }
>
> /*
> - * The init_user_ns is used to indicate that a vfsmount is not idmapped.
> - * This is simpler than just having to treat NULL as unmapped. Users
> - * wanting to idmap a mount to init_user_ns can just use a namespace
> - * with an identity mapping.
> + * The initial idmapping cannot be used to create an idmapped
> + * mount. Attaching the initial idmapping doesn't make much sense
> + * as it is an identity mapping. A user can just create a dedicated
> + * identity mapping to achieve the same result. We also use the
> + * initial idmapping as an indicator of a mount that is not
> + * idmapped. It can simply be passed into helpers that are aware of
> + * idmapped mounts as a convenient shortcut.
> */
This isn't completely accurate, is it? If sb->s_user_ns != init_user_ns,
an idmap to init_user_ns isn't an idendity mapping.
> mnt_userns = container_of(ns, struct user_namespace, ns);
> - if (mnt_userns == &init_user_ns) {
> + if (initial_mapping(mnt_userns)) {
> err = -EPERM;
> goto out_fput;
> }
> diff --git a/fs/open.c b/fs/open.c
> index 40a00e71865b..9ff2f621b760 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -641,7 +641,7 @@ SYSCALL_DEFINE2(chmod, const char __user *, filename, umode_t, mode)
>
> int chown_common(const struct path *path, uid_t user, gid_t group)
> {
> - struct user_namespace *mnt_userns;
> + struct user_namespace *mnt_userns, *fs_userns;
> struct inode *inode = path->dentry->d_inode;
> struct inode *delegated_inode = NULL;
> int error;
> @@ -653,8 +653,9 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
> gid = make_kgid(current_user_ns(), group);
>
> mnt_userns = mnt_user_ns(path->mnt);
> - uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
> - gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
> + fs_userns = i_user_ns(inode);
> + uid = mapped_kuid_user(mnt_userns, fs_userns, uid);
> + gid = mapped_kgid_user(mnt_userns, fs_userns, gid);
>
> retry_deleg:
> newattrs.ia_valid = ATTR_CTIME;
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 4b5fb9a9b90f..80acb6885cf9 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -376,8 +376,8 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
> break;
> case ACL_USER:
> uid = mapped_kuid_fs(mnt_userns,
> - &init_user_ns,
> - pa->e_uid);
> + i_user_ns(inode),
> + pa->e_uid);
> if (uid_eq(uid, current_fsuid()))
> goto mask;
> break;
> @@ -391,8 +391,8 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
> break;
> case ACL_GROUP:
> gid = mapped_kgid_fs(mnt_userns,
> - &init_user_ns,
> - pa->e_gid);
> + i_user_ns(inode),
> + pa->e_gid);
> if (in_group_p(gid)) {
> found = 1;
> if ((pa->e_perm & want) == want)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c7f72b78ab7e..9df6903634e8 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1641,7 +1641,7 @@ static inline void i_gid_write(struct inode *inode, gid_t gid)
> static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
> const struct inode *inode)
> {
> - return mapped_kuid_fs(mnt_userns, &init_user_ns, inode->i_uid);
> + return mapped_kuid_fs(mnt_userns, i_user_ns(inode), inode->i_uid);
> }
>
> /**
> @@ -1655,7 +1655,7 @@ static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
> static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
> const struct inode *inode)
> {
> - return mapped_kgid_fs(mnt_userns, &init_user_ns, inode->i_gid);
> + return mapped_kgid_fs(mnt_userns, i_user_ns(inode), inode->i_gid);
> }
>
> /**
> @@ -1669,7 +1669,7 @@ static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
> static inline void inode_fsuid_set(struct inode *inode,
> struct user_namespace *mnt_userns)
> {
> - inode->i_uid = mapped_fsuid(mnt_userns, &init_user_ns);
> + inode->i_uid = mapped_fsuid(mnt_userns, i_user_ns(inode));
> }
>
> /**
> @@ -1683,7 +1683,7 @@ static inline void inode_fsuid_set(struct inode *inode,
> static inline void inode_fsgid_set(struct inode *inode,
> struct user_namespace *mnt_userns)
> {
> - inode->i_gid = mapped_fsgid(mnt_userns, &init_user_ns);
> + inode->i_gid = mapped_fsgid(mnt_userns, i_user_ns(inode));
> }
>
> /**
> @@ -1704,10 +1704,10 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> kuid_t kuid;
> kgid_t kgid;
>
> - kuid = mapped_fsuid(mnt_userns, &init_user_ns);
> + kuid = mapped_fsuid(mnt_userns, fs_userns);
> if (!uid_valid(kuid))
> return false;
> - kgid = mapped_fsgid(mnt_userns, &init_user_ns);
> + kgid = mapped_fsgid(mnt_userns, fs_userns);
> if (!gid_valid(kgid))
> return false;
> return kuid_has_mapping(fs_userns, kuid) &&
> @@ -2654,13 +2654,14 @@ static inline struct user_namespace *file_mnt_user_ns(struct file *file)
> * is_idmapped_mnt - check whether a mount is mapped
> * @mnt: the mount to check
> *
> - * If @mnt has an idmapping attached to it @mnt is mapped.
> + * If @mnt has an idmapping attached different from the
> + * filesystem's idmapping then @mnt is mapped.
> *
> * Return: true if mount is mapped, false if not.
> */
> static inline bool is_idmapped_mnt(const struct vfsmount *mnt)
> {
> - return mnt_user_ns(mnt) != &init_user_ns;
> + return mnt_user_ns(mnt) != mnt->mnt_sb->s_user_ns;
> }
>
> extern long vfs_truncate(const struct path *, loff_t);
> diff --git a/security/commoncap.c b/security/commoncap.c
> index d288a62e2999..5fc8986c3c77 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -419,7 +419,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns,
> kroot = make_kuid(fs_ns, root);
>
> /* If this is an idmapped mount shift the kuid. */
> - kroot = mapped_kuid_fs(mnt_userns, &init_user_ns, kroot);
> + kroot = mapped_kuid_fs(mnt_userns, fs_ns, kroot);
>
> /* If the root kuid maps to a valid uid in current ns, then return
> * this as a nscap. */
> @@ -556,13 +556,12 @@ int cap_convert_nscap(struct user_namespace *mnt_userns, struct dentry *dentry,
> return -EINVAL;
> if (!capable_wrt_inode_uidgid(mnt_userns, inode, CAP_SETFCAP))
> return -EPERM;
> - if (size == XATTR_CAPS_SZ_2 && (mnt_userns == &init_user_ns))
> + if (size == XATTR_CAPS_SZ_2 && (mnt_userns == fs_ns))
> if (ns_capable(inode->i_sb->s_user_ns, CAP_SETFCAP))
> /* user is privileged, just write the v2 */
> return size;
>
> - rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns,
> - &init_user_ns);
> + rootid = rootid_from_xattr(*ivalue, size, task_ns, mnt_userns, fs_ns);
> if (!uid_valid(rootid))
> return -EINVAL;
>
> @@ -703,7 +702,7 @@ int get_vfs_caps_from_disk(struct user_namespace *mnt_userns,
> /* Limit the caps to the mounter of the filesystem
> * or the more limited uid specified in the xattr.
> */
> - rootkuid = mapped_kuid_fs(mnt_userns, &init_user_ns, rootkuid);
> + rootkuid = mapped_kuid_fs(mnt_userns, fs_ns, rootkuid);
> if (!rootid_owns_currentns(rootkuid))
> return -ENODATA;
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v2 10/10] fs: support mapped mounts of mapped filesystems
2021-12-02 17:50 ` Seth Forshee
@ 2021-12-02 19:20 ` Christian Brauner
0 siblings, 0 replies; 22+ messages in thread
From: Christian Brauner @ 2021-12-02 19:20 UTC (permalink / raw)
To: Seth Forshee
Cc: Christian Brauner, Christoph Hellwig, Amir Goldstein, Al Viro,
linux-fsdevel
On Thu, Dec 02, 2021 at 11:50:43AM -0600, Seth Forshee wrote:
> On Tue, Nov 30, 2021 at 01:10:32PM +0100, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> >
> > In previous patches we added new and modified existing helpers to handle
> > idmapped mounts of filesystems mounted with an idmapping. In this final
> > patch we convert all relevant places in the vfs to actually pass the
> > filesystem's idmapping into these helpers.
> >
> > With this the vfs is in shape to handle idmapped mounts of filesystems
> > mounted with an idmapping. Note that this is just the generic
> > infrastructure. Actually adding support for idmapped mounts to a
> > filesystem mountable with an idmapping is follow-up work.
> >
> > In this patch we extend the definition of an idmapped mount from a mount
> > that that has the initial idmapping attached to it to a mount that has
> > an idmapping attached to it which is not the same as the idmapping the
> > filesystem was mounted with.
> >
> > As before we do not allow the initial idmapping to be attached to a
> > mount. In addition this patch prevents that the idmapping the filesystem
> > was mounted with can be attached to a mount created based on this
> > filesystem.
> >
> > This has multiple reasons and advantages. First, attaching the initial
> > idmapping or the filesystem's idmapping doesn't make much sense as in
> > both cases the values of the i_{g,u}id and other places where k{g,u}ids
> > are used do not change. Second, a user that really wants to do this for
> > whatever reason can just create a separate dedicated identical idmapping
> > to attach to the mount. Third, we can continue to use the initial
> > idmapping as an indicator that a mount is not idmapped allowing us to
> > continue to keep passing the initial idmapping into the mapping helpers
> > to tell them that something isn't an idmapped mount even if the
> > filesystem is mounted with an idmapping.
> >
> > Link: https://lore.kernel.org/r/20211123114227.3124056-11-brauner@kernel.org (v1)
> > Cc: Seth Forshee <sforshee@digitalocean.com>
> > Cc: Amir Goldstein <amir73il@gmail.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > CC: linux-fsdevel@vger.kernel.org
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> > /* v2 */
> > - Amir Goldstein <amir73il@gmail.com>:
> > - Continue passing down the initial idmapping to xfs. Since xfs cannot
> > and is not planned to support idmapped superblocks don't mislead
> > readers in the code by passing down the filesystem idmapping. It
> > will be the initial idmapping always anyway.
> > - Include mnt_idmapping.h header into fs/namespace.c
> > ---
> > fs/namespace.c | 37 +++++++++++++++++++++++++------------
> > fs/open.c | 7 ++++---
> > fs/posix_acl.c | 8 ++++----
> > include/linux/fs.h | 17 +++++++++--------
> > security/commoncap.c | 9 ++++-----
> > 5 files changed, 46 insertions(+), 32 deletions(-)
> >
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 4994b816a74c..ccefede4ba1b 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -31,6 +31,7 @@
> > #include <uapi/linux/mount.h>
> > #include <linux/fs_context.h>
> > #include <linux/shmem_fs.h>
> > +#include <linux/mnt_idmapping.h>
> >
> > #include "pnode.h"
> > #include "internal.h"
> > @@ -561,7 +562,7 @@ static void free_vfsmnt(struct mount *mnt)
> > struct user_namespace *mnt_userns;
> >
> > mnt_userns = mnt_user_ns(&mnt->mnt);
> > - if (mnt_userns != &init_user_ns)
> > + if (!initial_mapping(mnt_userns))
> > put_user_ns(mnt_userns);
> > kfree_const(mnt->mnt_devname);
> > #ifdef CONFIG_SMP
> > @@ -965,6 +966,7 @@ static struct mount *skip_mnt_tree(struct mount *p)
> > struct vfsmount *vfs_create_mount(struct fs_context *fc)
> > {
> > struct mount *mnt;
> > + struct user_namespace *fs_userns;
> >
> > if (!fc->root)
> > return ERR_PTR(-EINVAL);
> > @@ -982,6 +984,10 @@ struct vfsmount *vfs_create_mount(struct fs_context *fc)
> > mnt->mnt_mountpoint = mnt->mnt.mnt_root;
> > mnt->mnt_parent = mnt;
> >
> > + fs_userns = mnt->mnt.mnt_sb->s_user_ns;
> > + if (!initial_mapping(fs_userns))
> > + mnt->mnt.mnt_userns = get_user_ns(fs_userns);
> > +
>
> Won't this get be leaked if mnt_userns is overwritten by
> do_idmap_mount()?
Yep, good catch. Thanks for spotting this! I fixed that up in the tree.
We need a put in do_idmap_mount() as you've mentioned for the
!initial_mapping() case.
>
> > lock_mount_hash();
> > list_add_tail(&mnt->mnt_instance, &mnt->mnt.mnt_sb->s_mounts);
> > unlock_mount_hash();
> > @@ -1072,7 +1078,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
> >
> > atomic_inc(&sb->s_active);
> > mnt->mnt.mnt_userns = mnt_user_ns(&old->mnt);
> > - if (mnt->mnt.mnt_userns != &init_user_ns)
> > + if (!initial_mapping(mnt->mnt.mnt_userns))
> > mnt->mnt.mnt_userns = get_user_ns(mnt->mnt.mnt_userns);
> > mnt->mnt.mnt_sb = sb;
> > mnt->mnt.mnt_root = dget(root);
> > @@ -3927,10 +3933,18 @@ static unsigned int recalc_flags(struct mount_kattr *kattr, struct mount *mnt)
> > static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
> > {
> > struct vfsmount *m = &mnt->mnt;
> > + struct user_namespace *fs_userns = m->mnt_sb->s_user_ns;
> >
> > if (!kattr->mnt_userns)
> > return 0;
> >
> > + /*
> > + * Creating an idmapped mount with the filesystem wide idmapping
> > + * doesn't make sense so block that. We don't allow mushy semantics.
> > + */
> > + if (kattr->mnt_userns == fs_userns)
> > + 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
> > @@ -3943,12 +3957,8 @@ static int can_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
> > if (!(m->mnt_sb->s_type->fs_flags & FS_ALLOW_IDMAP))
> > return -EINVAL;
> >
> > - /* Don't yet support filesystem mountable in user namespaces. */
> > - if (m->mnt_sb->s_user_ns != &init_user_ns)
> > - return -EINVAL;
> > -
> > /* We're not controlling the superblock. */
> > - if (!capable(CAP_SYS_ADMIN))
> > + if (!ns_capable(fs_userns, CAP_SYS_ADMIN))
> > return -EPERM;
> >
> > /* Mount has already been visible in the filesystem hierarchy. */
> > @@ -4133,13 +4143,16 @@ static int build_mount_idmapped(const struct mount_attr *attr, size_t usize,
> > }
> >
> > /*
> > - * The init_user_ns is used to indicate that a vfsmount is not idmapped.
> > - * This is simpler than just having to treat NULL as unmapped. Users
> > - * wanting to idmap a mount to init_user_ns can just use a namespace
> > - * with an identity mapping.
> > + * The initial idmapping cannot be used to create an idmapped
> > + * mount. Attaching the initial idmapping doesn't make much sense
> > + * as it is an identity mapping. A user can just create a dedicated
> > + * identity mapping to achieve the same result. We also use the
> > + * initial idmapping as an indicator of a mount that is not
> > + * idmapped. It can simply be passed into helpers that are aware of
> > + * idmapped mounts as a convenient shortcut.
> > */
>
> This isn't completely accurate, is it? If sb->s_user_ns != init_user_ns,
> an idmap to init_user_ns isn't an idendity mapping.
Yep, that's correct. Let me reword that.
Thank you for the reviews!
Christian
^ permalink raw reply [flat|nested] 22+ messages in thread