Re: [PATCH v5 1/4] fs: move sgid strip operation from inode_init_owner into inode_sgid_strip

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

于 2022/4/21 16:01, Christian Brauner 写道:
> On Thu, Apr 21, 2022 at 03:54:15PM +0800, Yang Xu wrote:
>> This has no functional change. Just create and export inode_sgid_strip
>> api for the subsequent patch. This function is used to strip inode's
>> S_ISGID mode when init a new inode.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>
> Could you please add the kernel doc I sketched below to the new helper?
>
> Looks good to me,
> Reviewed-by: Christian Brauner (Microsoft)<brauner@kernel.org>
>
>> v4-v5:
>> use umode_t return value instead of mode pointer
>>   fs/inode.c         | 23 +++++++++++++++++++----
>>   include/linux/fs.h |  2 ++
>>   2 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 9d9b422504d1..57130e4ef8b4 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -2246,10 +2246,8 @@ void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
>>   		/* Directories are special, and always inherit S_ISGID */
>>   		if (S_ISDIR(mode))
>>   			mode |= S_ISGID;
>> -		else if ((mode&  (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)&&
>> -			 !in_group_p(i_gid_into_mnt(mnt_userns, dir))&&
>> -			 !capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
>> -			mode&= ~S_ISGID;
>> +		else
>> +			mode = inode_sgid_strip(mnt_userns, dir, mode);
>>   	} else
>>   		inode_fsgid_set(inode, mnt_userns);
>>   	inode->i_mode = mode;
>> @@ -2405,3 +2403,20 @@ struct timespec64 current_time(struct inode *inode)
>>   	return timestamp_truncate(now, inode);
>>   }
>>   EXPORT_SYMBOL(current_time);
>> +
>
> /**
>   * inode_sgid_strip - handle the sgid bit for non-directories
>   * @mnt_userns:	idmapping of the mount
Maybe replace it with
"@mnt_userns： User namespace of the mount the inode was created from"?
>   * @dir: parent directory
parent directory inode
>   * @mode: mode of the file to be created in @dir
>   *
>   * If the @mode of the new file has both the S_ISGID and S_IXGRP bit
>   * raised and @dir has the S_ISGID bit raised ensure that the caller is
>   * either in the group of the parent directory or they have CAP_FSETID
>   * in their user namespace and are privileged over the parent directory.
>   * In all other cases, strip the S_ISGID bit from @mode.
>   *
>   * Return: the new mode to use for the file
>   */
>> +umode_t inode_sgid_strip(struct user_namespace *mnt_userns,
>> +			 const struct inode *dir, umode_t mode)
>> +{
>> +	if (S_ISDIR(mode) || !dir || !(dir->i_mode&  S_ISGID))
>> +		return mode;
>> +	if ((mode&  (S_ISGID | S_IXGRP)) != (S_ISGID | S_IXGRP))
>> +		return mode;
>> +	if (in_group_p(i_gid_into_mnt(mnt_userns, dir)))
>> +		return mode;
>> +	if (capable_wrt_inode_uidgid(mnt_userns, dir, CAP_FSETID))
>> +		return mode;
>> +
>> +	mode&= ~S_ISGID;
>> +	return mode;
>> +}
>> +EXPORT_SYMBOL(inode_sgid_strip);
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index bbde95387a23..532de76c9b91 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1897,6 +1897,8 @@ extern long compat_ptr_ioctl(struct file *file, unsigned int cmd,
>>   void inode_init_owner(struct user_namespace *mnt_userns, struct inode *inode,
>>   		      const struct inode *dir, umode_t mode);
>>   extern bool may_open_dev(const struct path *path);
>> +umode_t inode_sgid_strip(struct user_namespace *mnt_userns,
>> +			 const struct inode *dir, umode_t mode);
>>
>>   /*
>>    * This is the "filldir" function type, used by readdir() to let
>> --
>> 2.27.0
>>