[PATCH v3 2/5] attr: add setattr_should_drop_sgid()

[Christian Brauner <brauner@kernel.org>]

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.

Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---

Notes:
    /* v2 */
    Dave Chinner <dchinner@redhat.com>:
    - Use easier to follow logic in the new helper.
    
    /* v3 */
    Amir Goldstein <amir73il@gmail.com>:
    - Rename helper from should_remove_sgid() to setattr_should_drop_sgid() to
      better indicate its semantics.
    - Return setattr_should_drop_sgid() directly now that it returns ATTR_KILL_SGID
      instead of a boolean.

 fs/attr.c     | 26 ++++++++++++++++++++++++++
 fs/internal.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/fs/attr.c b/fs/attr.c
index 8bc2edd6bd3c..3d03ceb332e5 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -39,6 +39,32 @@ static int setattr_drop_sgid(struct user_namespace *mnt_userns,
 	return ATTR_KILL_SGID;
 }
 
+/**
+ * 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;
+	return setattr_drop_sgid(mnt_userns, inode,
+				 i_gid_into_vfsgid(mnt_userns, inode));
+}
+
 /**
  * chown_ok - verify permissions to chown inode
  * @mnt_userns:	user namespace of the mount @inode was found from
diff --git a/fs/internal.h b/fs/internal.h
index 6f0386b34fae..988e123d3885 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -234,3 +234,5 @@ int do_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 		struct xattr_ctx *ctx);
 
 ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *pos);
+int setattr_should_drop_sgid(struct user_namespace *mnt_userns,
+			     const struct inode *inode);
-- 
2.34.1