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

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

 



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?

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.




[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