[Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately

[Kirill Tkhai]

On 25.12.2018 12:45, Pavel Butsykin wrote:
> 
> 
> On 25.12.2018 12:28, Kirill Tkhai wrote:
>> On 24.12.2018 15:54, Pavel Butsykin wrote:
>>> Fallocate without mutex lock can race with setattr size request, as a result,
>>> may be various problems, including incorrectly changed file size. At the same
>>> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can immediately
>>
>> man 2 fallocate:
>>
>>    "Specifying  the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15) in mode zeros space in the byte range starting at offset and
>>     continuing for len bytes.  Within the specified range, blocks are preallocated for the regions that span the  holes  in  the  file.
>>     After a successful call, subsequent reads from this range will return zeros".
> 
> Yes, FALLOC_FL_ZERO_RANGE | FALLOC_FL_ZERO_RANGE mode combinations
> should always be under mutex, so we can just move my check before:
> 		if (inarg->offset >= di->fileinfo.attr.size)
> 			inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
> 
>> So, this patch makes pcs_fuse_kdirect() complete a request without error
>> and without content changing, while userspace thinks the range was zeroed.
>> It's wrong.
>>
>>> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move mutex_is_locked,
>>> since all other mode combinations either have to be under mutex or not valid.
>>>
>>> #VSTOR-19317
>>>
>>> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
>>> ---
>>>   fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> index de54fedeb5e4..89866370a341 100644
>>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>>>   		if (inarg->offset >= di->fileinfo.attr.size)
>>>   			inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>>>   
>>> +		if (inarg->mode == FALLOC_FL_KEEP_SIZE)
>>> +			break; /* NOPE */
>>
>> This is untidy to add new branch, which makes further "if (inarg->mode & FALLOC_FL_KEEP_SIZE)"
>> check a dead code.
> 
> Why? mode = FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE won't pass my
> check, but will pass the check "if (inarg->mode & FALLOC_FL_KEEP_SIZE)",
> it's not a dead code.

Oh, I misread that. Yeah, there is no problem.

>>> +
>>> +		WARN_ON_ONCE(!mutex_is_locked(&fi->inode.i_mutex));
>>>   		if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
>>> -			WARN_ON_ONCE(!mutex_is_locked(&fi->inode.i_mutex));
>>> -
>>>   			if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length & (PAGE_SIZE - 1))) {
>>>   				r->req.out.h.error = -EINVAL;
>>>   				goto error;
>>>