Re: [PATCH v8 1/4] fs: add mode_strip_sgid() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 19, 2022 at 01:03:12AM +0000, xuyang2018.jy@xxxxxxxxxxx wrote:
> on 2022/4/28 17:34, Christian Brauner wrote:
> > On Thu, Apr 28, 2022 at 03:46:59AM +0000, Al Viro wrote:
> >> On Thu, Apr 28, 2022 at 03:12:30AM +0000, Al Viro wrote:
> >>
> >>>> Note, BTW, that while XFS has inode_fsuid_set() on the non-inode_init_owner()
> >>>> path, it doesn't have inode_fsgid_set() there.  Same goes for ext4, while
> >>>> ext2 doesn't bother with either in such case...
> >>>>
> >>>> Let's try to separate the issues here.  Jann, could you explain what makes
> >>>> empty sgid files dangerous?
> >>>
> >>> Found the original thread in old mailbox, and the method of avoiding the
> >>> SGID removal on modification is usable.  Which answers the question above...
> >>
> >> OK, what do we want for grpid mounts?  Aside of "don't forget inode_fsuid_set(),
> >> please", that is.  We don't want inode_fsgid_set() there (whatever went for
> >> the parent directory should be the right value for the child).  Same "strip
> >
> > Exactly. You sounded puzzled why we don't call that in an earlier mail.
> >
> >> SGID from non-directory child, unless we are in the resulting group"?
> >
> > Honestly, I think we should try and see if we can't use the same setgid
> > inheritance enforcement of the new mode_strip_sgid() helper for the
> > grpid mount option as well. Iow, just don't give the grpid mount option
> > a separate setgid treatment and try it with the current approach.
> >
> > It'll allow us to move things into vfs proper which I think is a robust
> > solution with clear semantics. It also gives us a uniform ordering wrt
> > to umask stripping and POSIX ACLs.
> >
> > Yes, as we've pointed out in the thread this carries a non-zero
> > regression risk. But so does the whole patch series. But this might end
> > up being a big win security wise and makes maintenance way easier going
> > forward.
> >
> > The current setgid situation is thoroughly messy though and we keep
> > plugging holes. Even writing tests for the current situation is an
> > almost herculean task let alone reviewing it.
> 
> Sorry for the late reply.
> I am agree with these. So what should I do in next step?

I'd say we try to go forward with the original approach and not
special-casing the grpid option.

> 
> ps:I also think I may send test case to xfstests for posix acl and umask 
> ASAP, then xfstests can merge these test and so more people will notice 
> this problem.

The big refactoring of the idmapped mounts testsuite into a generic
vfstest refactoring is sitting in for-next of xfstests so make sure to
base your patches on that because you really won't enjoy having to
rebase them...



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux