on 2022/4/14 20:02, Christian Brauner 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@xxxxxxxxxx> >> Signed-off-by: Yang Xu<xuyang2018.jy@xxxxxxxxxxx> >> --- >> 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? Dave's comment as below: " Agreed, that's a much nicer API for this function - it makes it clear that it can modifying the mode that is passed in. " So I think Dave should like modify mode directly instead of returning a umode_t value. @Dave So which way do you mean? Best Regards Yang Xu > > 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; > }