Re: [PATCH v2 1/3] vfs: Add inode_sgid_strip() api

[Christian Brauner <brauner@kernel.org>]

On Thu, Apr 14, 2022 at 03:57:17PM +0800, Yang Xu wrote:
> inode_sgid_strip() function is used to strip S_ISGID mode
> when creat/open/mknod file.
> 
> Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  fs/inode.c         | 18 ++++++++++++++++++
>  include/linux/fs.h |  3 ++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 9d9b422504d1..d63264998855 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2405,3 +2405,21 @@ struct timespec64 current_time(struct inode *inode)
>  	return timestamp_truncate(now, inode);
>  }
>  EXPORT_SYMBOL(current_time);
> +
> +void inode_sgid_strip(struct user_namespace *mnt_userns, struct inode *dir,
> +		      umode_t *mode)
> +{
> +	if (!dir || !(dir->i_mode & S_ISGID))
> +		return;
> +	if ((*mode & (S_ISGID | S_IXGRP)) != (S_ISGID | S_IXGRP))
> +		return;
> +	if (S_ISDIR(*mode))
> +		return;
> +	if (in_group_p(i_gid_into_mnt(mnt_userns, dir)))
> +		return;
> +	if (capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
> +		return;
> +
> +	*mode &= ~S_ISGID;
> +}
> +EXPORT_SYMBOL(inode_sgid_strip);


I still think this should return umode_t with the setgid bit stripped
instead of modifying the mode directly. I may have misunderstood Dave,
but I thought he preferred to return umode_t too?

umode_t inode_sgid_strip(struct user_namespace *mnt_userns, struct inode *dir, umode_t mode)
{
	if (S_ISDIR(mode))
		return mode;

	if (!dir || !(dir->i_mode & S_ISGID))
		return;

	if ((mode & (S_ISGID | S_IXGRP)) != (S_ISGID | S_IXGRP))
		return;

	if (in_group_p(i_gid_into_mnt(mnt_userns, dir)))
		return;

	if (capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
		return;

	return mode & ~S_ISGID;
}