Skip to content

Commit

Permalink
attr: add should_remove_sgid()
Browse files Browse the repository at this point in the history
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
should_remove_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>
  • Loading branch information
brauner authored and intel-lab-lkp committed Oct 7, 2022
1 parent c8580ea commit d18cf34
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
27 changes: 27 additions & 0 deletions fs/attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,33 @@ static bool setattr_drop_sgid(struct user_namespace *mnt_userns,
return true;
}

/**
* should_remove_sgid - determine whether the setgid bit needs to be removed
* @mnt_userns: User namespace of the mount the inode was created 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 should_remove_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 (setattr_drop_sgid(mnt_userns, inode,
i_gid_into_vfsgid(mnt_userns, inode)))
return ATTR_KILL_SGID;
return 0;
}

/**
* chown_ok - verify permissions to chown inode
* @mnt_userns: user namespace of the mount @inode was found from
Expand Down
2 changes: 2 additions & 0 deletions fs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,5 @@ ssize_t do_getxattr(struct user_namespace *mnt_userns,
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);
int should_remove_sgid(struct user_namespace *mnt_userns,
const struct inode *inode);

0 comments on commit d18cf34

Please sign in to comment.