On Fri, Apr 15, 2022 at 07:02:17PM +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. > > Signed-off-by: Yang Xu <xuyang2018.jy@xxxxxxxxxxx> > --- > v2->v3: > 1.Use const struct inode * instead of struct inode * > 2.replace sgid strip with inode_sgid_strip in a single patch > fs/inode.c | 24 ++++++++++++++++++++---- > include/linux/fs.h | 3 ++- > 2 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index 9d9b422504d1..1b569ad882ce 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,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, > + const 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; I'd place that check first as this whole function is really only relevant for non-directories. Otherwise I can live with *mode being a pointer although I still find this unpleasant API wise but the bikeshed does it's job without having my color. :) I'd like to do some good testing on this. Acked-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>