Re: [PATCH] ceph: update the time stamps and try to drop the suid/sgid

[Xiubo Li <xiubli@redhat.com>]

On 13/02/2023 20:37, Jeff Layton wrote:
> On Mon, 2023-02-13 at 19:10 +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> The fallocate will try to clear the suid/sgid if a unprevileged user
>> changed the file.
>>
>> There is no Posix item requires that we should clear the suid/sgid
>> in fallocate code path but this is the default behaviour for most of
>> the filesystems and the VFS layer. And also the same for the write
>> code path, which have already support it.
>>
> Huh, you're right. It really doesn't say anything about the timestamps
> or setuid bits:
>
>      https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_fallocate.html
>
>
> That's arguably a bug in the spec. It really does need to do those
> things.

Yeah.

Also the kernel fuse code and libfuse also need to be improved to make 
ceph-fuse work.

Thanks Jeff.

- Xiubo

>> And also we need to update the time stamps since the fallocate will
>> change the file contents.
>>
>> Cc: stable@vger.kernel.org
>> URL: https://tracker.ceph.com/issues/58054
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>   fs/ceph/file.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 903de296f0d3..dee3b445f415 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -2502,6 +2502,9 @@ static long ceph_fallocate(struct file *file, int mode,
>>   	loff_t endoff = 0;
>>   	loff_t size;
>>   
>> +	dout("%s %p %llx.%llx mode %x, offset %llu length %llu\n", __func__,
>> +	     inode, ceph_vinop(inode), mode, offset, length);
>> +
>>   	if (mode != (FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>>   		return -EOPNOTSUPP;
>>   
>> @@ -2539,6 +2542,10 @@ static long ceph_fallocate(struct file *file, int mode,
>>   	if (ret < 0)
>>   		goto unlock;
>>   
>> +	ret = file_modified(file);
>> +	if (ret)
>> +		goto put_caps;
>> +
>>   	filemap_invalidate_lock(inode->i_mapping);
>>   	ceph_fscache_invalidate(inode, false);
>>   	ceph_zero_pagecache_range(inode, offset, length);
>> @@ -2554,6 +2561,7 @@ static long ceph_fallocate(struct file *file, int mode,
>>   	}
>>   	filemap_invalidate_unlock(inode->i_mapping);
>>   
>> +put_caps:
>>   	ceph_put_cap_refs(ci, got);
>>   unlock:
>>   	inode_unlock(inode);
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>
-- 
Best Regards,

Xiubo Li (李秀波)

Email: xiubli@redhat.com/xiubli@ibm.com
Slack: @Xiubo Li