Re: [PATCH v7 4/4] ceph: Remove S_ISGID stripping code in ceph_finish_async_create

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

 



on 2022/4/26 15:14, Christian Brauner wrote:
> On Tue, Apr 26, 2022 at 12:19:52PM +0800, Yang Xu wrote:
>> Previous patches moved sgid stripping exclusively into the vfs. So
>> manual sgid stripping by the filesystem isn't needed anymore.
>>
>> Reviewed-by: Xiubo Li<xiubli@xxxxxxxxxx>
>> Reviewed-by: Christian Brauner (Microsoft)<brauner@xxxxxxxxxx>
>> Signed-off-by: Yang Xu<xuyang2018.jy@xxxxxxxxxxx>
>> ---
>
> Since this is a very sensitive patch series I think we need to be
> annoyingly pedantic about the commit messages. This is really only
> necessary because of the nature of these changes so you'll forgive me
> for being really annoying about this. Here's what I'd change the commit
> messages to:
>
> ceph: rely on vfs for setgid stripping
>
> Now that we finished moving setgid stripping for regular files in setgid
> directories into the vfs, individual filesystem don't need to manually
> strip the setgid bit anymore. Drop the now unneeded code from ceph.

This seems better, thanks.
>
>
>>   fs/ceph/file.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 6c9e837aa1d3..8e3b99853333 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -651,10 +651,6 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
>>   		/* Directories always inherit the setgid bit. */
>>   		if (S_ISDIR(mode))
>>   			mode |= S_ISGID;
>
> (Frankly, this ideally shouldn't be necessary as well, i.e. it'd be
> great if that part would've been done by the vfs already too but it's
> not as security sensitive as setgid stripping for regular files.)

Maybe we can just add mode_add_sgid api into vfs_prepare_mode in the 
future or only just add mode_add_sgid into do_mkdirat?

static inline umode_t vfs_prepare_mode(struct user_namespace *mnt_userns,
				   const struct inode *dir, umode_t mode)
{
	mode = mode_strip_sgid(mnt_userns, dir, mode);

	if (!IS_POSIXACL(dir))
		mode &= ~current_umask();

        mode = mode_add_sgid(dir, mode)

        return mode;
}

fs/inode.c
umodet mode_add_sgid(const struct inode *dir, umode_t mode)
{
	if (dir && dir->i_mode & S_ISGID && S_ISDIR(mode))
		 mode |= S_ISGID;

	return mode;		
}


Then we can remove  "mode |= S_ISGID" code in inode_init_owner after we 
check all places called inode_init_owner.

But now, let's finished S_ISGID stripping patchset firstly.




[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