FAILED: patch "[PATCH] fs: account for group membership" failed to apply to 5.15-stable tree
Subsystems (12)
[email protected]
maintainerThe patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 168f912893407a5acb798a4a58613b5f1f98c717 Mon Sep 17 00:00:00 2001
From: Christian Brauner <[email protected]>
Date: Mon, 13 Jun 2022 13:15:17 +0200
Subject: [PATCH] fs: account for group membership
When calling setattr_prepare() to determine the validity of the attributes the ia_{g,u}id fields contain the value that will be written to inode->i_{g,u}id. This is exactly the same for idmapped and non-idmapped mounts and allows callers to pass in the values they want to see written to inode->i_{g,u}id.
When group ownership is changed a caller whose fsuid owns the inode can change the group of the inode to any group they are a member of. When searching through the caller's groups we need to use the gid mapped according to the idmapped mount otherwise we will fail to change ownership for unprivileged users.
Consider a caller running with fsuid and fsgid 1000 using an idmapped mount that maps id 65534 to 1000 and 65535 to 1001. Consequently, a file owned by 65534:65535 in the filesystem will be owned by 1000:1001 in the idmapped mount.
The caller now requests the gid of the file to be changed to 1000 going through the idmapped mount. In the vfs we will immediately map the requested gid to the value that will need to be written to inode->i_gid and place it in attr->ia_gid. Since this idmapped mount maps 65534 to 1000 we place 65534 in attr->ia_gid.
When we check whether the caller is allowed to change group ownership we first validate that their fsuid matches the inode's uid. The inode->i_uid is 65534 which is mapped to uid 1000 in the idmapped mount. Since the caller's fsuid is 1000 we pass the check.
We now check whether the caller is allowed to change inode->i_gid to the requested gid by calling in_group_p(). This will compare the passed in gid to the caller's fsgid and search the caller's additional groups.
Since we're dealing with an idmapped mount we need to pass in the gid mapped according to the idmapped mount. This is akin to checking whether a caller is privileged over the future group the inode is owned by. And that needs to take the idmapped mount into account. Note, all helpers are nops without idmapped mounts.
New regression test sent to xfstests.
fs/attr.c +20 -6
@@ -61,9 +61,15 @@ static bool chgrp_ok(struct user_namespace *mnt_userns, const struct inode *inode, kgid_t gid) {kgid_t kgid =i_gid_into_mnt (mnt_userns, inode); -if (uid_eq (current_fsuid (),i_uid_into_mnt (mnt_userns, inode)) && - (in_group_p (gid) ||gid_eq (gid, inode->i_gid))) -return true; +if (uid_eq (current_fsuid (),i_uid_into_mnt (mnt_userns, inode))) { +kgid_t mapped_gid; + +if (gid_eq (gid, inode->i_gid)) +return true; + mapped_gid =mapped_kgid_fs (mnt_userns,i_user_ns (inode), gid); +if (in_group_p (mapped_gid)) +return true; + }if (capable_wrt_inode_uidgid (mnt_userns, inode,CAP_CHOWN ))return true; if (gid_eq(kgid, INVALID_GID) &&
@@ -123,12 +129,20 @@ int setattr_prepare(struct user_namespace *mnt_userns, struct dentry *dentry,/* Make sure a caller can chmod. */ if (ia_valid &ATTR_MODE ) { +kgid_t mapped_gid; +if (!inode_owner_or_capable (mnt_userns, inode))return -EPERM ; + +if (ia_valid &ATTR_GID ) + mapped_gid =mapped_kgid_fs (mnt_userns, +i_user_ns (inode), attr->ia_gid); +else + mapped_gid =i_gid_into_mnt (mnt_userns, inode); +/* Also check the setgid bit! */ -if (!in_group_p ((ia_valid &ATTR_GID ) ? attr->ia_gid : -i_gid_into_mnt (mnt_userns, inode)) && - !capable_wrt_inode_uidgid (mnt_userns, inode,CAP_FSETID )) +if (!in_group_p (mapped_gid) && + !capable_wrt_inode_uidgid (mnt_userns, inode,CAP_FSETID )) attr->ia_mode &= ~S_ISGID ; }
Christian Brauner
maintainerQuoted 5 lines · On Mon, Jun 20, 2022 at 11:56:54AM +0200, [email protected] wrote:
The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Hm, I just tried on top of v5.15.50:
git cherry-pick -S 168f912893407a5acb798a4a5
and it applied cleanly. Can you try and backport this again, please? Or tell me how to reproduce the failure you're seeing so I can fix it and give you an applicable version?
Thanks!
Christian
Christian Brauner
maintainerQuoted 7 lines · Christian Brauner
Quoted 5 lines · On Mon, Jun 20, 2022 at 11:56:54AM +0200, [email protected] wrote:
The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 7 lines
Hm, I just tried on top of v5.15.50:
git cherry-pick -S
168f912893407a5acb798a4a5and it applied cleanly. Can you try and backport this again, please? Or tell me how to reproduce the failure you're seeing so I can fix it and give you an applicable version?
It's a build problem. I'll give you a series that makes this patch apply. I'll backport a few more patches if you don't mind.
Thanks!
Christian
Christian Brauner
maintainercover attr: group fix backport
From: "Christian Brauner (Microsoft)" <[email protected]>
Hey Greg,
As promised, here is a series that allows to backport the fix which failed to build for you. This backports a few patches that are required to make this work. I decided to backport them instead of rolling a custom fix for this. That would've been smaller but there is future hardening work that I would like to backport and this enables this.
I've run xfstests for ext4, xfs, and btrfs as well as LTP with:
runltp -f fs_perms_simple,fs_bind,containers,cap_bounds,cve,uevent,filecaps and I see no regressions. There is an xfs failure but that is related to a - for obvious reasons - missing stable backport.
Thanks!
Christian
Christian Brauner (12):
fs: add is_idmapped_mnt() helper
fs: move mapping helpers
fs: tweak fsuidgid_has_mapping()
fs: account for filesystem mappings
docs: update mapping documentation
fs: use low-level mapping helpers
fs: remove unused low-level mapping helpers
fs: port higher-level mapping helpers
fs: add i_user_ns() helper
fs: support mapped mounts of mapped filesystems
fs: fix acl translation
fs: account for group membership
Documentation/filesystems/idmappings.rst | 72 ------- fs/attr.c | 26 ++- fs/cachefiles/bind.c | 2 +- fs/ecryptfs/main.c | 2 +- fs/ksmbd/smbacl.c | 19 +- fs/ksmbd/smbacl.h | 5 +- fs/namespace.c | 53 +++-- fs/nfsd/export.c | 2 +- fs/open.c | 8 +- fs/overlayfs/super.c | 2 +- fs/posix_acl.c | 27 ++- fs/proc_namespace.c | 2 +- fs/xattr.c | 6 +- fs/xfs/xfs_inode.c | 8 +- fs/xfs/xfs_linux.h | 1 + fs/xfs/xfs_symlink.c | 4 +- include/linux/fs.h | 141 ++++---------- include/linux/mnt_idmapping.h | 234 +++++++++++++++++++++++ include/linux/posix_acl_xattr.h | 4 + security/commoncap.c | 15 +- 20 files changed, 394 insertions(+), 239 deletions(-) create mode 100644 include/linux/mnt_idmapping.h
base-commit: 18a33c8dabb88b50b860e0177a73933f2c0ddf68
Signature · 1 line
2.34.1
Greg KH
maintainerRe: [PATCH 00/12] attr: group fix backport
Quoted 10 lines · Christian Brauner
From: "Christian Brauner (Microsoft)" <[email protected]>
Hey Greg,
As promised, here is a series that allows to backport the fix which failed to build for you. This backports a few patches that are required to make this work. I decided to backport them instead of rolling a custom fix for this. That would've been smaller but there is future hardening work that I would like to backport and this enables this.
Quoted 4 lines
I've run xfstests for ext4, xfs, and btrfs as well as LTP with:
runltp -f fs_perms_simple,fs_bind,containers,cap_bounds,cve,uevent,filecaps and I see no regressions. There is an xfs failure but that is related to a - for obvious reasons - missing stable backport.
Thanks for these, all now queued up.
greg k-h
Christian Brauner
maintainerPATCH 1/12 fs: add is_idmapped_mnt() helper
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit bb49e9e730c2906a958eee273a7819f401543d6c upstream.
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.
--- 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(-)
fs/cachefiles/bind.c +1 -1
@@ -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; }
fs/ecryptfs/main.c +1 -1
@@ -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;
fs/namespace.c +1 -1
@@ -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. */
fs/nfsd/export.c +1 -1
@@ -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 ; }
fs/overlayfs/super.c +1 -1
@@ -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; }
fs/proc_namespace.c +1 -1
@@ -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" ); }
include/linux/fs.h +14 -0
@@ -2726,6 +2726,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);
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 2/12 fs: move mapping helpers
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit a793d79ea3e041081cd7cbd8ee43d0b5e4914a2b upstream.
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.
--- 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
fs/ksmbd/smbacl.c +1 -0
@@ -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"
fs/ksmbd/smbacl.h +1 -0
@@ -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"
fs/open.c +1 -0
@@ -32,6 +32,7 @@#include <linux/ima.h> #include <linux/dnotify.h> #include <linux/compat.h> +#include <linux/mnt_idmapping.h> #include "internal.h"
fs/posix_acl.c +1 -0
@@ -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) {
fs/xfs/xfs_linux.h +1 -0
@@ -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>
include/linux/fs.h +1 -90
109 lines not on this page · raw message
include/linux/mnt_idmapping.h new file +101 -0
101 lines not on this page · raw message
security/commoncap.c +1 -0
@@ -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
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 3/12 fs: tweak fsuidgid_has_mapping()
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 476860b3eb4a50958243158861d5340066df5af2 upstream.
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.
--- include/linux/fs.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
include/linux/fs.h +11 -3
@@ -1697,10 +1697,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);
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 4/12 fs: account for filesystem mappings
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 1ac2a4104968e0a60b4b3572216a92aab5c1b025 upstream.
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_idmapping() 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")
--- include/linux/fs.h | 4 +- include/linux/mnt_idmapping.h | 193 +++++++++++++++++++++++++++++++++- 2 files changed, 191 insertions(+), 6 deletions(-)
include/linux/fs.h +2 -2
@@ -1638,7 +1638,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); }/**
@@ -1652,7 +1652,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); }/**
include/linux/mnt_idmapping.h +189 -4
223 lines not on this page · raw message
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 5/12 docs: update mapping documentation
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 8cc5c54de44c5e8e104d364a627ac4296845fc7f upstream.
Now that we implement the full remapping algorithms described in our documentation remove the section about shortcircuting them.
--- Documentation/filesystems/idmappings.rst | 72 ------------------------ 1 file changed, 72 deletions(-)
Documentation/filesystems/idmappings.rst +0 -72
@@ -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.
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 6/12 fs: use low-level mapping helpers
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 4472071331549e911a5abad41aea6e3be855a1a4 upstream.
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.
--- 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(-)
fs/ksmbd/smbacl.c +2 -16
@@ -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 ;
fs/ksmbd/smbacl.h +2 -2
@@ -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);
fs/open.c +2 -2
@@ -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 ;
fs/posix_acl.c +10 -6
@@ -376,7 +376,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 ;
@@ -389,7 +391,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)
@@ -736,17 +740,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 :
security/commoncap.c +8 -5
@@ -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 ;
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 7/12 fs: remove unused low-level mapping helpers
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 02e4079913500f24ceb082d8d87d8665f044b298 upstream.
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 commit a65e58e791a1 ("fs: document and rename fsid helpers").
--- include/linux/mnt_idmapping.h | 56 ----------------------------------- 1 file changed, 56 deletions(-)
include/linux/mnt_idmapping.h +0 -56
@@ -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_idmapping - check whether this is the initial mapping * @ns: idmapping to check
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 8/12 fs: port higher-level mapping helpers
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit 209188ce75d0d357c292f6bb81d712acdd4e7db7 upstream.
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.
--- 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(-)
fs/xfs/xfs_inode.c +4 -4
@@ -994,8 +994,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)
@@ -1148,8 +1148,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)
fs/xfs/xfs_symlink.c +2 -2
@@ -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)
include/linux/fs.h +4 -4
@@ -1666,7 +1666,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); }/**
@@ -1680,7 +1680,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); }/**
@@ -1701,10 +1701,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) &&
include/linux/mnt_idmapping.h +8 -4
@@ -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_IDMAPPING_H */
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 9/12 fs: add i_user_ns() helper
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit a1ec9040a2a9122605ac26e5725c6de019184419 upstream.
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.
--- include/linux/fs.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
include/linux/fs.h +9 -4
@@ -1602,6 +1602,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
@@ -1609,22 +1614,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); }/**
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 10/12 fs: support mapped mounts of mapped filesystems
In reply to Christian BraunerFrom: Christian Brauner <[email protected]>
commit bd303368b776eead1c29e6cdda82bde7128b82a7 upstream.
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.
--- fs/namespace.c | 51 +++++++++++++++++++++++++++++++++----------- fs/open.c | 7 +++--- fs/posix_acl.c | 8 +++---- include/linux/fs.h | 17 ++++++++------- security/commoncap.c | 9 ++++---- 5 files changed, 59 insertions(+), 33 deletions(-)
fs/namespace.c +38 -13
@@ -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_idmapping (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_idmapping (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_idmapping (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. */
@@ -4002,14 +4012,27 @@ static struct mount *mount_setattr_prepare(struct mount_kattr *kattr,static void do_idmap_mount (const struct mount_kattr *kattr,struct mount *mnt) { -struct user_namespace *mnt_userns; +struct user_namespace *mnt_userns, *old_mnt_userns;if (!kattr->mnt_userns)return ; +/* +* We're the only ones able to change the mount's idmapping. So +* mnt->mnt.mnt_userns is stable and we can retrieve it directly. +*/ + old_mnt_userns = mnt->mnt.mnt_userns; + mnt_userns =get_user_ns (kattr->mnt_userns);/* Pairs with smp_load_acquire() in mnt_user_ns(). */ smp_store_release (&mnt->mnt.mnt_userns, mnt_userns); + +/* +* If this is an idmapped filesystem drop the reference we've taken +* in vfs_create_mount() before. +*/ +if (!initial_idmapping (old_mnt_userns)) +put_user_ns (old_mnt_userns); } static void mount_setattr_commit(struct mount_kattr *kattr,
@@ -4133,13 +4156,15 @@ 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. We 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. A user +* can just create a dedicated identity mapping to achieve the same +* result. */ mnt_userns = container_of(ns, struct user_namespace, ns); -if (mnt_userns == &init_user_ns) { +if (initial_idmapping (mnt_userns)) { err = -EPERM ; goto out_fput; }
fs/open.c +4 -3
@@ -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 ;
fs/posix_acl.c +4 -4
@@ -377,8 +377,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 ;
@@ -392,8 +392,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)
include/linux/fs.h +9 -8
@@ -1643,7 +1643,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); }/**
@@ -1657,7 +1657,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); }/**
@@ -1671,7 +1671,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)); }/**
@@ -1685,7 +1685,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)); }/**
@@ -1706,10 +1706,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) &&
@@ -2655,13 +2655,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 );
security/commoncap.c +4 -5
@@ -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 ;
Signature · 1 line
2.34.1
Christian Brauner
maintainerPATCH 11/12 fs: fix acl translation
In reply to Christian Braunercommit 705191b03d507744c7e097f78d583621c14988ac upstream.
Last cycle we extended the idmapped mounts infrastructure to support idmapped mounts of idmapped filesystems (No such filesystem yet exist.). Since then, the meaning of an idmapped mount is a mount whose idmapping is different from the filesystems idmapping.
While doing that work we missed to adapt the acl translation helpers. They still assume that checking for the identity mapping is enough. But they need to use the no_idmapping() helper instead.
Note, POSIX ACLs are always translated right at the userspace-kernel boundary using the caller's current idmapping and the initial idmapping. The order depends on whether we're coming from or going to userspace. The filesystem's idmapping doesn't matter at the border.
Consequently, if a non-idmapped mount is passed we need to make sure to always pass the initial idmapping as the mount's idmapping and not the filesystem idmapping. Since it's irrelevant here it would yield invalid ids and prevent setting acls for filesystems that are mountable in a userns and support posix acls (tmpfs and fuse).
I verified the regression reported in [1] and verified that this patch fixes it. A regression test will be added to xfstests in parallel.
--- fs/posix_acl.c | 10 ++++++++++ fs/xattr.c | 6 ++++-- include/linux/posix_acl_xattr.h | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-)
fs/posix_acl.c +10 -0
@@ -760,9 +760,14 @@ static void posix_acl_fix_xattr_userns( }void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) {struct user_namespace *user_ns =current_user_ns (); + +/* Leave ids untouched on non-idmapped mounts. */ +if (no_idmapping (mnt_userns,i_user_ns (inode))) + mnt_userns = &init_user_ns;if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))return ; posix_acl_fix_xattr_userns(&init_user_ns, user_ns, mnt_userns, value,
@@ -770,9 +775,14 @@ void posix_acl_fix_xattr_from_user(struct user_namespace *mnt_userns, }void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) {struct user_namespace *user_ns =current_user_ns (); + +/* Leave ids untouched on non-idmapped mounts. */ +if (no_idmapping (mnt_userns,i_user_ns (inode))) + mnt_userns = &init_user_ns;if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))return ; posix_acl_fix_xattr_userns(user_ns, &init_user_ns, mnt_userns, value,
fs/xattr.c +4 -2
@@ -569,7 +569,8 @@ setxattr(struct user_namespace *mnt_userns, struct dentry *d, }if ((strcmp (kname,XATTR_NAME_POSIX_ACL_ACCESS ) ==0 ) || (strcmp (kname,XATTR_NAME_POSIX_ACL_DEFAULT ) ==0 )) -posix_acl_fix_xattr_from_user (mnt_userns, kvalue, size); +posix_acl_fix_xattr_from_user (mnt_userns,d_inode (d), + kvalue, size); } error =vfs_setxattr (mnt_userns, d, kname, kvalue, size, flags);
@@ -667,7 +668,8 @@ getxattr(struct user_namespace *mnt_userns, struct dentry *d,if (error >0 ) {if ((strcmp (kname,XATTR_NAME_POSIX_ACL_ACCESS ) ==0 ) || (strcmp (kname,XATTR_NAME_POSIX_ACL_DEFAULT ) ==0 )) -posix_acl_fix_xattr_to_user (mnt_userns, kvalue, error); +posix_acl_fix_xattr_to_user (mnt_userns,d_inode (d), + kvalue, error);if (size &©_to_user (value, kvalue, error)) error = -EFAULT ; }else if (error == -ERANGE && size >=XATTR_SIZE_MAX ) {
include/linux/posix_acl_xattr.h +4 -0
@@ -34,15 +34,19 @@ posix_acl_xattr_count(size_t size)#ifdef CONFIG_FS_POSIX_ACL void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size);void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size);#else static inline void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) { }static inline void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) { }
Signature · 1 line
2.34.1
Patch "fs: fix acl translation" has been added to the 5.15-stable tree
This is a note to let you know that I've just added the patch titled
fs: fix acl translation
to the 5.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
fs-fix-acl-translation.patch
and it can be found in the queue-5.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
From foo@baz Thu Jun 30 01:35:30 PM CEST 2022
From: Christian Brauner <[email protected]>
Date: Tue, 28 Jun 2022 14:16:19 +0200
Subject: fs: fix acl translation
To: Greg KH <[email protected]>
Cc: Christian Brauner <[email protected]>, [email protected]
Message-ID: <[email protected]>
From: Christian Brauner <[email protected]>
commit 705191b03d507744c7e097f78d583621c14988ac upstream.
Last cycle we extended the idmapped mounts infrastructure to support idmapped mounts of idmapped filesystems (No such filesystem yet exist.). Since then, the meaning of an idmapped mount is a mount whose idmapping is different from the filesystems idmapping.
While doing that work we missed to adapt the acl translation helpers. They still assume that checking for the identity mapping is enough. But they need to use the no_idmapping() helper instead.
Note, POSIX ACLs are always translated right at the userspace-kernel boundary using the caller's current idmapping and the initial idmapping. The order depends on whether we're coming from or going to userspace. The filesystem's idmapping doesn't matter at the border.
Consequently, if a non-idmapped mount is passed we need to make sure to always pass the initial idmapping as the mount's idmapping and not the filesystem idmapping. Since it's irrelevant here it would yield invalid ids and prevent setting acls for filesystems that are mountable in a userns and support posix acls (tmpfs and fuse).
I verified the regression reported in [1] and verified that this patch fixes it. A regression test will be added to xfstests in parallel.
--- fs/posix_acl.c | 10 ++++++++++ fs/xattr.c | 6 ++++-- include/linux/posix_acl_xattr.h | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-)
fs/posix_acl.c +10 -0
@@ -760,9 +760,14 @@ static void posix_acl_fix_xattr_userns( }void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) {struct user_namespace *user_ns =current_user_ns (); + +/* Leave ids untouched on non-idmapped mounts. */ +if (no_idmapping (mnt_userns,i_user_ns (inode))) + mnt_userns = &init_user_ns;if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))return ; posix_acl_fix_xattr_userns(&init_user_ns, user_ns, mnt_userns, value,
@@ -770,9 +775,14 @@ void posix_acl_fix_xattr_from_user(struc }void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) {struct user_namespace *user_ns =current_user_ns (); + +/* Leave ids untouched on non-idmapped mounts. */ +if (no_idmapping (mnt_userns,i_user_ns (inode))) + mnt_userns = &init_user_ns;if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))return ; posix_acl_fix_xattr_userns(user_ns, &init_user_ns, mnt_userns, value,
fs/xattr.c +4 -2
@@ -569,7 +569,8 @@ setxattr(struct user_namespace *mnt_user }if ((strcmp (kname,XATTR_NAME_POSIX_ACL_ACCESS ) ==0 ) || (strcmp (kname,XATTR_NAME_POSIX_ACL_DEFAULT ) ==0 )) -posix_acl_fix_xattr_from_user (mnt_userns, kvalue, size); +posix_acl_fix_xattr_from_user (mnt_userns,d_inode (d), + kvalue, size); } error =vfs_setxattr (mnt_userns, d, kname, kvalue, size, flags);
@@ -667,7 +668,8 @@ getxattr(struct user_namespace *mnt_userif (error >0 ) {if ((strcmp (kname,XATTR_NAME_POSIX_ACL_ACCESS ) ==0 ) || (strcmp (kname,XATTR_NAME_POSIX_ACL_DEFAULT ) ==0 )) -posix_acl_fix_xattr_to_user (mnt_userns, kvalue, error); +posix_acl_fix_xattr_to_user (mnt_userns,d_inode (d), + kvalue, error);if (size &©_to_user (value, kvalue, error)) error = -EFAULT ; }else if (error == -ERANGE && size >=XATTR_SIZE_MAX ) {
include/linux/posix_acl_xattr.h +4 -0
@@ -34,15 +34,19 @@ posix_acl_xattr_count(size_t size)#ifdef CONFIG_FS_POSIX_ACL void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size);void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size);#else static inline void posix_acl_fix_xattr_from_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) { }static inline void posix_acl_fix_xattr_to_user (struct user_namespace *mnt_userns, +struct inode *inode,void *value,size_t size) { }
Patches currently in stable-queue which might be from [email protected] are
queue-5.15/fs-account-for-group-membership.patch queue-5.15/fs-use-low-level-mapping-helpers.patch queue-5.15/fs-move-mapping-helpers.patch queue-5.15/fs-remove-unused-low-level-mapping-helpers.patch queue-5.15/fs-tweak-fsuidgid_has_mapping.patch queue-5.15/fs-add-i_user_ns-helper.patch queue-5.15/fs-add-is_idmapped_mnt-helper.patch queue-5.15/fs-support-mapped-mounts-of-mapped-filesystems.patch queue-5.15/fs-fix-acl-translation.patch queue-5.15/fs-port-higher-level-mapping-helpers.patch queue-5.15/docs-update-mapping-documentation.patch queue-5.15/fs-account-for-filesystem-mappings.patch
Christian Brauner
maintainerPATCH 12/12 fs: account for group membership
In reply to Christian Braunercommit 168f912893407a5acb798a4a58613b5f1f98c717 upstream.
When calling setattr_prepare() to determine the validity of the attributes the ia_{g,u}id fields contain the value that will be written to inode->i_{g,u}id. This is exactly the same for idmapped and non-idmapped mounts and allows callers to pass in the values they want to see written to inode->i_{g,u}id.
When group ownership is changed a caller whose fsuid owns the inode can change the group of the inode to any group they are a member of. When searching through the caller's groups we need to use the gid mapped according to the idmapped mount otherwise we will fail to change ownership for unprivileged users.
Consider a caller running with fsuid and fsgid 1000 using an idmapped mount that maps id 65534 to 1000 and 65535 to 1001. Consequently, a file owned by 65534:65535 in the filesystem will be owned by 1000:1001 in the idmapped mount.
The caller now requests the gid of the file to be changed to 1000 going through the idmapped mount. In the vfs we will immediately map the requested gid to the value that will need to be written to inode->i_gid and place it in attr->ia_gid. Since this idmapped mount maps 65534 to 1000 we place 65534 in attr->ia_gid.
When we check whether the caller is allowed to change group ownership we first validate that their fsuid matches the inode's uid. The inode->i_uid is 65534 which is mapped to uid 1000 in the idmapped mount. Since the caller's fsuid is 1000 we pass the check.
We now check whether the caller is allowed to change inode->i_gid to the requested gid by calling in_group_p(). This will compare the passed in gid to the caller's fsgid and search the caller's additional groups.
Since we're dealing with an idmapped mount we need to pass in the gid mapped according to the idmapped mount. This is akin to checking whether a caller is privileged over the future group the inode is owned by. And that needs to take the idmapped mount into account. Note, all helpers are nops without idmapped mounts.
New regression test sent to xfstests.
--- fs/attr.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)
fs/attr.c +20 -6
@@ -61,9 +61,15 @@ static bool chgrp_ok(struct user_namespace *mnt_userns, const struct inode *inode, kgid_t gid) {kgid_t kgid =i_gid_into_mnt (mnt_userns, inode); -if (uid_eq (current_fsuid (),i_uid_into_mnt (mnt_userns, inode)) && - (in_group_p (gid) ||gid_eq (gid, inode->i_gid))) -return true; +if (uid_eq (current_fsuid (),i_uid_into_mnt (mnt_userns, inode))) { +kgid_t mapped_gid; + +if (gid_eq (gid, inode->i_gid)) +return true; + mapped_gid =mapped_kgid_fs (mnt_userns,i_user_ns (inode), gid); +if (in_group_p (mapped_gid)) +return true; + }if (capable_wrt_inode_uidgid (mnt_userns, inode,CAP_CHOWN ))return true; if (gid_eq(kgid, INVALID_GID) &&
@@ -123,12 +129,20 @@ int setattr_prepare(struct user_namespace *mnt_userns, struct dentry *dentry,/* Make sure a caller can chmod. */ if (ia_valid &ATTR_MODE ) { +kgid_t mapped_gid; +if (!inode_owner_or_capable (mnt_userns, inode))return -EPERM ; + +if (ia_valid &ATTR_GID ) + mapped_gid =mapped_kgid_fs (mnt_userns, +i_user_ns (inode), attr->ia_gid); +else + mapped_gid =i_gid_into_mnt (mnt_userns, inode); +/* Also check the setgid bit! */ -if (!in_group_p ((ia_valid &ATTR_GID ) ? attr->ia_gid : -i_gid_into_mnt (mnt_userns, inode)) && - !capable_wrt_inode_uidgid (mnt_userns, inode,CAP_FSETID )) +if (!in_group_p (mapped_gid) && + !capable_wrt_inode_uidgid (mnt_userns, inode,CAP_FSETID )) attr->ia_mode &= ~S_ISGID ; }
Signature · 1 line
2.34.1