All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Seth Forshee <sforshee@digitalocean.com>,
	Christoph Hellwig <hch@lst.de>, Aleksa Sarai <cyphar@cyphar.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	"Christian Brauner (Microsoft)" <brauner@kernel.org>
Subject: [PATCH 5.15 24/28] fs: account for group membership
Date: Thu, 30 Jun 2022 15:47:20 +0200	[thread overview]
Message-ID: <20220630133233.642494810@linuxfoundation.org> (raw)
In-Reply-To: <20220630133232.926711493@linuxfoundation.org>

From: Christian Brauner <brauner@kernel.org>

commit 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.

Link: https://github.com/lxc/lxd/issues/10537
Link: https://lore.kernel.org/r/20220613111517.2186646-1-brauner@kernel.org
Fixes: 2f221d6f7b88 ("attr: handle idmapped mounts")
Cc: Seth Forshee <sforshee@digitalocean.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: stable@vger.kernel.org # 5.15+
CC: linux-fsdevel@vger.kernel.org
Reviewed-by: Seth Forshee <sforshee@digitalocean.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/attr.c |   26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

--- a/fs/attr.c
+++ b/fs/attr.c
@@ -61,9 +61,15 @@ static bool chgrp_ok(struct user_namespa
 		     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_namespac
 
 	/* 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;
 	}
 



  parent reply	other threads:[~2022-06-30 14:10 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-30 13:46 [PATCH 5.15 00/28] 5.15.52-rc1 review Greg Kroah-Hartman
2022-06-30 13:46 ` [PATCH 5.15 01/28] tick/nohz: unexport __init-annotated tick_nohz_full_setup() Greg Kroah-Hartman
2022-06-30 13:46 ` [PATCH 5.15 02/28] clocksource/drivers/ixp4xx: remove __init from ixp4xx_timer_setup() Greg Kroah-Hartman
2022-07-01 15:31   ` Nathan Chancellor
2022-07-01 15:50     ` Greg Kroah-Hartman
2022-06-30 13:46 ` [PATCH 5.15 03/28] x86, kvm: use proper ASM macros for kvm_vcpu_is_preempted Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 04/28] bcache: memset on stack variables in bch_btree_check() and bch_sectors_dirty_init() Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 05/28] xfs: use kmem_cache_free() for kmem_cache objects Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 06/28] xfs: punch out data fork delalloc blocks on COW writeback failure Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 07/28] xfs: Fix the free logic of state in xfs_attr_node_hasname Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 08/28] xfs: remove all COW fork extents when remounting readonly Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 09/28] xfs: check sb_meta_uuid for dabuf buffer recovery Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 10/28] xfs: prevent UAF in xfs_log_item_in_current_chkpt Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 11/28] xfs: only bother with sync_filesystem during readonly remount Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 12/28] powerpc/ftrace: Remove ftrace init tramp once kernel init is complete Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 13/28] fs: add is_idmapped_mnt() helper Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 14/28] fs: move mapping helpers Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 15/28] fs: tweak fsuidgid_has_mapping() Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 16/28] fs: account for filesystem mappings Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 17/28] docs: update mapping documentation Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 18/28] fs: use low-level mapping helpers Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 19/28] fs: remove unused " Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 20/28] fs: port higher-level " Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 21/28] fs: add i_user_ns() helper Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 22/28] fs: support mapped mounts of mapped filesystems Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 23/28] fs: fix acl translation Greg Kroah-Hartman
2022-06-30 13:47 ` Greg Kroah-Hartman [this message]
2022-06-30 13:47 ` [PATCH 5.15 25/28] rtw88: 8821c: support RFE type4 wifi NIC Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 26/28] rtw88: rtw8821c: enable rfe 6 devices Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 27/28] net: mscc: ocelot: allow unregistered IP multicast flooding to CPU Greg Kroah-Hartman
2022-06-30 13:47 ` [PATCH 5.15 28/28] io_uring: fix not locked access to fixed buf table Greg Kroah-Hartman
2022-06-30 17:09 ` [PATCH 5.15 00/28] 5.15.52-rc1 review Jon Hunter
2022-06-30 23:17 ` Shuah Khan
2022-06-30 23:21 ` Florian Fainelli
2022-07-01  0:58 ` Guenter Roeck
2022-07-01  3:51 ` Bagas Sanjaya
2022-07-01  3:51   ` Bagas Sanjaya
2022-07-01  6:18 ` Naresh Kamboju
2022-07-01  8:51 ` Christian Brauner
2022-07-01  8:57   ` Greg Kroah-Hartman
2022-07-01 12:51     ` Christian Brauner
2022-07-01 10:36 ` Sudip Mukherjee
2022-07-01 13:55 ` Ron Economos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220630133233.642494810@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=brauner@kernel.org \
    --cc=cyphar@cyphar.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sforshee@digitalocean.com \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.