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

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

on 2022/4/14 23:57, Darrick J. Wong wrote:
> 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;
>> +}
>
> Thanks for cleaning up the multiple if statements from last time.
>
> I still would like to see patch 1 replace the code in inode_init_owner
> so that we can compare before and after in the same patch.  Patch 2 can
> then be solely about moving the callsite around the VFS.
>
Ok, then patch 1 can named as"fs/inode: move inode sgid strip operation 
from inode_init_owner into inode_sgid_strip".  What do you think about it?


Best Regards
Yang Xu
> --D
>
>> +EXPORT_SYMBOL(inode_sgid_strip);
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index bbde95387a23..94d94219fe7c 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1897,7 +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);
>> -
>> +void inode_sgid_strip(struct user_namespace *mnt_userns, struct inode *dir,
>> +		      umode_t *mode);
>>   /*
>>    * This is the "filldir" function type, used by readdir() to let
>>    * the kernel specify what kind of dirent layout it wants to have.
>> --
>> 2.27.0
>>