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

[Stanislav Kinsburskiy]

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

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