linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leah Rumancik <leah.rumancik@gmail.com>
To: stable@vger.kernel.org
Cc: linux-xfs@vger.kernel.org, amir73il@gmail.com,
	chandan.babu@oracle.com, Christian Brauner <brauner@kernel.org>,
	Leah Rumancik <leah.rumancik@gmail.com>,
	"Darrick J . Wong" <djwong@kernel.org>
Subject: [PATCH 5.15 09/11] attr: add setattr_should_drop_sgid()
Date: Tue,  7 Mar 2023 10:59:20 -0800	[thread overview]
Message-ID: <20230307185922.125907-10-leah.rumancik@gmail.com> (raw)
In-Reply-To: <20230307185922.125907-1-leah.rumancik@gmail.com>

From: Christian Brauner <brauner@kernel.org>

commit 72ae017c5451860443a16fb2a8c243bff3e396b8 upstream.

[backport to 5.15.y, prior to vfsgid_t]

The current setgid stripping logic during write and ownership change
operations is inconsistent and strewn over multiple places. In order to
consolidate it and make more consistent we'll add a new helper
setattr_should_drop_sgid(). The function retains the old behavior where
we remove the S_ISGID bit unconditionally when S_IXGRP is set but also
when it isn't set and the caller is neither in the group of the inode
nor privileged over the inode.

We will use this helper both in write operation permission removal such
as file_remove_privs() as well as in ownership change operations.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Tested-by: Leah Rumancik <leah.rumancik@gmail.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/attr.c     | 28 ++++++++++++++++++++++++++++
 fs/internal.h |  6 ++++++
 2 files changed, 34 insertions(+)

diff --git a/fs/attr.c b/fs/attr.c
index f045431bab1a..965be68ed8fa 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -20,6 +20,34 @@
 
 #include "internal.h"
 
+/**
+ * setattr_should_drop_sgid - determine whether the setgid bit needs to be
+ *                            removed
+ * @mnt_userns:	user namespace of the mount @inode was found from
+ * @inode:	inode to check
+ *
+ * This function determines whether the setgid bit needs to be removed.
+ * We retain backwards compatibility and require setgid bit to be removed
+ * unconditionally if S_IXGRP is set. Otherwise we have the exact same
+ * requirements as setattr_prepare() and setattr_copy().
+ *
+ * Return: ATTR_KILL_SGID if setgid bit needs to be removed, 0 otherwise.
+ */
+int setattr_should_drop_sgid(struct user_namespace *mnt_userns,
+			     const struct inode *inode)
+{
+	umode_t mode = inode->i_mode;
+
+	if (!(mode & S_ISGID))
+		return 0;
+	if (mode & S_IXGRP)
+		return ATTR_KILL_SGID;
+	if (!in_group_or_capable(mnt_userns, inode,
+				 i_gid_into_mnt(mnt_userns, inode)))
+		return ATTR_KILL_SGID;
+	return 0;
+}
+
 /*
  * The logic we want is
  *
diff --git a/fs/internal.h b/fs/internal.h
index c89814727281..45cf31d7380b 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -231,3 +231,9 @@ struct xattr_ctx {
 int setxattr_copy(const char __user *name, struct xattr_ctx *ctx);
 int do_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 		struct xattr_ctx *ctx);
+
+/*
+ * fs/attr.c
+ */
+int setattr_should_drop_sgid(struct user_namespace *mnt_userns,
+			     const struct inode *inode);
-- 
2.40.0.rc0.216.gc4246ad0f0-goog


  parent reply	other threads:[~2023-03-07 19:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-07 18:59 [PATCH 5.15 00/11] sgid fixes for 5.15.y Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 01/11] xfs: use setattr_copy to set vfs inode attributes Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 02/11] xfs: remove XFS_PREALLOC_SYNC Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 03/11] xfs: fallocate() should call file_modified() Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 04/11] xfs: set prealloc flag in xfs_alloc_file_space() Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 05/11] fs: add mode_strip_sgid() helper Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 06/11] fs: move S_ISGID stripping into the vfs_*() helpers Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 07/11] attr: add in_group_or_capable() Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 08/11] fs: move should_remove_suid() Leah Rumancik
2023-03-07 18:59 ` Leah Rumancik [this message]
2023-03-07 18:59 ` [PATCH 5.15 10/11] attr: use consistent sgid stripping checks Leah Rumancik
2023-03-07 18:59 ` [PATCH 5.15 11/11] fs: use consistent setgid checks in is_sxid() Leah Rumancik
2023-03-14  2:16 ` [PATCH 5.15 00/11] sgid fixes for 5.15.y Sasha Levin

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=20230307185922.125907-10-leah.rumancik@gmail.com \
    --to=leah.rumancik@gmail.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=chandan.babu@oracle.com \
    --cc=djwong@kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).