[Devel] [PATCH v3] fs/fuse kio: fix stack overrun in request_end()

[Kirill Tkhai]

On 31.05.2019 14:05, Pavel Butsykin wrote:
> Unexpectedly, request_end() function turned out to be recursive, which leads to
> stack overrun:
> [76293.902783]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
> [76293.902789]  [<ffffffffc0fdc9d3>] pcs_fuse_submit+0x49d/0x4f5 [fuse_kio_pcs]
> [76293.902795]  [<ffffffffc0fbd775>] ? fuse_flush_writepages+0x8f/0x8f [fuse]
> [76293.902800]  [<ffffffffc0fdd745>] kpcs_req_send+0x210/0x3bb [fuse_kio_pcs]
> [76293.902805]  [<ffffffffc0faa17b>] flush_bg_queue+0x1ba/0x20a [fuse]
> [76293.902810]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
> [76293.902816]  [<ffffffffc0fdc9d3>] pcs_fuse_submit+0x49d/0x4f5 [fuse_kio_pcs]
> [76293.902818]  [<ffffffff8c57cdd0>] ? debug_check_no_locks_freed+0x2a0/0x2a0
> [76293.902824]  [<ffffffffc0fbd775>] ? fuse_flush_writepages+0x8f/0x8f [fuse]
> [76293.902829]  [<ffffffffc0fdd745>] kpcs_req_send+0x210/0x3bb [fuse_kio_pcs]
> [76293.902834]  [<ffffffffc0faa17b>] flush_bg_queue+0x1ba/0x20a [fuse]
> [76293.902839]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
> 
> To fix this, let's prohibit doing bg flush inside pcs_fuse_submit() and
> kpcs_req_classify().
> 
> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>

Acked-by: Kirill Tkhai <ktkhai at virtuozzo.com>

> ---
>  fs/fuse/dev.c                      | 7 ++++---
>  fs/fuse/fuse_i.h                   | 6 +++++-
>  fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 +++---
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index b8a524f04298..16143d05111c 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -434,7 +434,7 @@ static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
>   * the 'end' callback is called if given, else the reference to the
>   * request is released
>   */
> -void request_end(struct fuse_conn *fc, struct fuse_req *req)
> +void __request_end(struct fuse_conn *fc, struct fuse_req *req, bool flush_bg)
>  {
>  	struct fuse_iqueue *fiq = req->fiq;
>  	bool bg;
> @@ -481,7 +481,8 @@ void request_end(struct fuse_conn *fc, struct fuse_req *req)
>  		}
>  		fc->num_background--;
>  		fc->active_background--;
> -		flush_bg_queue(fc, fiq);
> +		if (flush_bg)
> +			flush_bg_queue(fc, fiq);
>  		spin_unlock(&fc->bg_lock);
>  	}
>  	if (req->end) {
> @@ -493,7 +494,7 @@ void request_end(struct fuse_conn *fc, struct fuse_req *req)
>  		wake_up(&req->waitq);
>  	fuse_put_request(fc, req);
>  }
> -EXPORT_SYMBOL_GPL(request_end);
> +EXPORT_SYMBOL_GPL(__request_end);
>  
>  static void queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
>  {
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 841edaf20323..8f3456b57d13 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1110,7 +1110,11 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
>  		    struct file *file);
>  
>  void fuse_set_initialized(struct fuse_conn *fc);
> -void request_end(struct fuse_conn *fc, struct fuse_req *req);
> +void __request_end(struct fuse_conn *fc, struct fuse_req *req, bool flush_bg);
> +static inline void request_end(struct fuse_conn *fc, struct fuse_req *req)
> +{
> +	__request_end(fc, req, true);
> +}
>  struct fuse_req *fuse_generic_request_alloc(struct fuse_conn *fc,
>  					    struct kmem_cache *cachep,
>  					    unsigned npages, gfp_t flags);
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index 4dad29418e0d..acba12ede5f3 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -1052,7 +1052,7 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>  error:
>  	DTRACE("do fuse_request_end req:%p op:%d err:%d\n", req, req->in.h.opcode, req->out.h.error);
>  
> -	request_end(pfc->fc, req);
> +	__request_end(pfc->fc, req, false);
>  	return;
>  
>  submit:
> @@ -1249,7 +1249,7 @@ static int kpcs_req_classify(struct fuse_conn* fc, struct fuse_req *req,
>  		req->out.h.error = ret;
>  		if (lk)
>  			spin_unlock(&fc->bg_lock);
> -		request_end(fc, req);
> +		__request_end(fc, req, false);
>  		if (lk)
>  			spin_lock(&fc->bg_lock);
>  		return ret;
> @@ -1270,7 +1270,7 @@ static void kpcs_req_send(struct fuse_conn* fc, struct fuse_req *req,
>  	TRACE("Send req:%p op:%d end:%p bg:%d\n",
>  		req, req->in.h.opcode, req->end, bg);
>  
> -	/* request_end below will do fuse_put_request() */
> +	/* __request_end below will do fuse_put_request() */
>  	if (!bg)
>  		atomic_inc(&req->count);
>  	__clear_bit(FR_PENDING, &req->flags);
>