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

[Pavel Butsykin]

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".
> 
> 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.

I don't quite understand what case you're considering, which breaks
after my patch. fallocate with mode = 
FALLOC_FL_KEEP_SIZE|FALLOC_FL_ZERO_RANGE
and offset > file_size, should have no effect on the file.

>> 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.
> 
>> +
>> +		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;
>>