In setattr_{copy,prepare}() we need to perform the same permission checks to determine whether we need to drop the setgid bit or not. Instead of open-coding it twice add a simple helper the encapsulates the logic. We will reuse this helpers to make dropping the setgid bit during write operations more consistent in a follow up patch. Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx> --- Notes: /* v2 */ patch added /* v3 */ Amir Goldstein <amir73il@xxxxxxxxx>: - Return 0 or ATTR_KILL_SGID to make all dropping helpers behave similarly. fs/attr.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/fs/attr.c b/fs/attr.c index 1552a5f23d6b..8bc2edd6bd3c 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -18,6 +18,27 @@ #include <linux/evm.h> #include <linux/ima.h> +/** + * setattr_drop_sgid - check generic setgid permissions + * @mnt_userns: user namespace of the mount @inode was found from + * @inode: inode to check + * @vfsgid: the new/current vfsgid of @inode + * + * This function determines whether the setgid bit needs to be removed because + * the caller lacks privileges over the inode. + * + * Return: ATTR_KILL_SGID if the setgid bit needs to be removed, 0 if not. + */ +static int setattr_drop_sgid(struct user_namespace *mnt_userns, + const struct inode *inode, vfsgid_t vfsgid) +{ + if (vfsgid_in_group_p(vfsgid)) + return 0; + if (capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID)) + return 0; + return ATTR_KILL_SGID; +} + /** * chown_ok - verify permissions to chown inode * @mnt_userns: user namespace of the mount @inode was found from @@ -140,8 +161,7 @@ int setattr_prepare(struct user_namespace *mnt_userns, struct dentry *dentry, vfsgid = i_gid_into_vfsgid(mnt_userns, inode); /* Also check the setgid bit! */ - if (!vfsgid_in_group_p(vfsgid) && - !capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID)) + if (setattr_drop_sgid(mnt_userns, inode, vfsgid)) attr->ia_mode &= ~S_ISGID; } @@ -251,9 +271,8 @@ void setattr_copy(struct user_namespace *mnt_userns, struct inode *inode, inode->i_ctime = attr->ia_ctime; if (ia_valid & ATTR_MODE) { umode_t mode = attr->ia_mode; - vfsgid_t vfsgid = i_gid_into_vfsgid(mnt_userns, inode); - if (!vfsgid_in_group_p(vfsgid) && - !capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID)) + if (setattr_drop_sgid(mnt_userns, inode, + i_gid_into_vfsgid(mnt_userns, inode))) mode &= ~S_ISGID; inode->i_mode = mode; } -- 2.34.1