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

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.

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