[Devel] [PATCH 2/2] fuse: handle only fatal signals while waiting request answer

[Konstantin Khorenko]

On 10/17/2016 03:01 PM, Stanislav Kinsburskiy wrote:
> Kostya, please, modify patch comment with the following substitution:
>
>
> s/
> it doesn't drop pending signal flag/
> it doesn't drop pending signal flag, if processes is traced/
> g

done, thank you.

>
> Thanks.
>
> 13.10.2016 12:03, Stanislav Kinsburskiy пишет:
>> This patch is backport of 7d3a07fcb8a0d5c06718de14fb91fdf1ef20a0e2 commit.
>> Although this commit loks more like cleanup, it's not.
>> It fixes the issue with signals, which can lead to abort of page read in fuse,
>> called from page fault, immediatly leading to SIGBUS sent to the caller.
>> The issue is in block_sigs implementation: it doesn't drop pending signal
>> flag, which interrupts wait_event_interruptible in request_wait_answer, while
>> it's supposed to be interrupted by SIGKILL only.
>> IOW, any signal, arrived to the process, which does page fault handling on
>> fuse file, _before_ request_wait_answer is called, will lead to request
>> interruption, producing SIGBUS error in page fault handler (filemap_fault).
>>
>> https://jira.sw.ru/browse/PSBM-53581
>>
>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>> ---
>>   fs/fuse/dev.c |   42 ++++++++++++++++--------------------------
>>   1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> index a0d0b1a..0b6d000 100644
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -99,19 +99,6 @@ void fuse_request_free(struct fuse_req *req)
>>   	kmem_cache_free(fuse_req_cachep, req);
>>   }
>>
>> -static void block_sigs(sigset_t *oldset)
>> -{
>> -	sigset_t mask;
>> -
>> -	siginitsetinv(&mask, sigmask(SIGKILL));
>> -	sigprocmask(SIG_BLOCK, &mask, oldset);
>> -}
>> -
>> -static void restore_sigs(sigset_t *oldset)
>> -{
>> -	sigprocmask(SIG_SETMASK, oldset, NULL);
>> -}
>> -
>>   void __fuse_get_request(struct fuse_req *req)
>>   {
>>   	atomic_inc(&req->count);
>> @@ -144,15 +131,9 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn *fc, unsigned npages,
>>   	atomic_inc(&fc->num_waiting);
>>
>>   	if (fuse_block_alloc(fc, for_background)) {
>> -		sigset_t oldset;
>> -		int intr;
>> -
>> -		block_sigs(&oldset);
>> -		intr = wait_event_interruptible_exclusive(fc->blocked_waitq,
>> -				!fuse_block_alloc(fc, for_background));
>> -		restore_sigs(&oldset);
>>   		err = -EINTR;
>> -		if (intr)
>> +		if (wait_event_killable_exclusive(fc->blocked_waitq,
>> +				!fuse_block_alloc(fc, for_background)))
>>   			goto out;
>>   	}
>>
>> @@ -412,6 +393,19 @@ __acquires(fc->lock)
>>   	spin_lock(&fc->lock);
>>   }
>>
>> +static void wait_answer_killable(struct fuse_conn *fc,
>> +				 struct fuse_req *req)
>> +__releases(fc->lock)
>> +__acquires(fc->lock)
>> +{
>> +	if (fatal_signal_pending(current))
>> +		return;
>> +
>> +	spin_unlock(&fc->lock);
>> +	wait_event_killable(req->waitq, req->state == FUSE_REQ_FINISHED);
>> +	spin_lock(&fc->lock);
>> +}
>> +
>>   static void queue_interrupt(struct fuse_conn *fc, struct fuse_req *req)
>>   {
>>   	list_add_tail(&req->intr_entry, &fc->interrupts);
>> @@ -438,12 +432,8 @@ __acquires(fc->lock)
>>   	}
>>
>>   	if (!req->force) {
>> -		sigset_t oldset;
>> -
>>   		/* Only fatal signals may interrupt this */
>> -		block_sigs(&oldset);
>> -		wait_answer_interruptible(fc, req);
>> -		restore_sigs(&oldset);
>> +		wait_answer_killable(fc, req);
>>
>>   		if (req->aborted)
>>   			goto aborted;
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at openvz.org
>> https://lists.openvz.org/mailman/listinfo/devel
>
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
>