on 2022/4/19 22:05, Christian Brauner wrote:
> On Tue, Apr 19, 2022 at 07:47:07PM +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 S_ISGID mode when init
>> a new inode.
>>
>> Acked-by: Christian Brauner (Microsoft)<brauner@xxxxxxxxxx>
>> Signed-off-by: Yang Xu<xuyang2018.jy@xxxxxxxxxxx>
>> ---
>> fs/inode.c | 22 ++++++++++++++++++----
>> include/linux/fs.h | 3 ++-
>> 2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 9d9b422504d1..3215e61a0021 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
>> + inode_sgid_strip(mnt_userns, dir,&mode);
>> } else
>> inode_fsgid_set(inode, mnt_userns);
>> inode->i_mode = mode;
>> @@ -2405,3 +2403,19 @@ 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,
>> + const struct inode *dir, umode_t *mode)
>> +{
>
> I think with Willy agreeing in an earlier version with me and you
> needing to resend anyway I'd say have this return umode_t instead of
> passing a pointer.
IMO, I am fine with your and Willy way. But I need a reason otherwise
I can't convince myself why not use mode pointer directly.
I have asked you and Willy before why return umode_t value is better,
why not modify mode pointer directly? Since we have use mode as
argument, why not modify mode pointer directly in function?
Also the function name(inode_sgid_strip and prepare_mode) has expressed
their function clearly.
