[Devel] [PATCH] fuse kio: Fix deadlock in kpcs_req_send()

[Pavel Butsykin]

On 09.10.2018 19:04, Kirill Tkhai wrote:
> spin_lock(&fc->lock);
> flush_bg_queue()
>    fc->kio.op->req_send(fc, req, true, true)
>      pcs_kio_classify_req()
>        request_end()
>          spin_lock(&fc->lock);
>          ===> Deadlock
> 
> Also, pcs_kio_setattr_handle() may sleep.
> 
> [This should go on top of "[PATCH RFC] fs/fuse kio_pcs: fix double free of synchronous requests"]
> 
> https://pmc.acronis.com/browse/VSTOR-15924
> 
> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>

Reviewed-by: Pavel Butsykin <pbutsykin at virtuozzo.com>

Actually FUSE_SETATTR can't be async, but extra precaution won't hurt..

> ---
>   fs/fuse/kio/pcs/pcs_fuse_kdirect.c |   23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index c967b99ae116..06e254cc3ef9 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -1035,9 +1035,6 @@ static void pcs_kio_setattr_handle(struct fuse_inode *fi, struct fuse_req *req)
>   	struct fuse_setattr_in *inarg = (void*) req->in.args[0].value;
>   	struct pcs_dentry_info *di;
>   
> -	if (!(inarg->valid & FATTR_SIZE))
> -		return;
> -
>   	BUG_ON(!fi);
>   
>   	di = pcs_inode_from_fuse(fi);
> @@ -1063,7 +1060,7 @@ static void pcs_kio_setattr_handle(struct fuse_inode *fi, struct fuse_req *req)
>   		req->end = _pcs_grow_end;
>   }
>   
> -static int pcs_kio_classify_req(struct fuse_conn *fc, struct fuse_req *req)
> +static int pcs_kio_classify_req(struct fuse_conn *fc, struct fuse_req *req, bool lk)
>   {
>   	struct fuse_inode *fi = get_fuse_inode(req->io_inode);
>   
> @@ -1074,12 +1071,20 @@ static int pcs_kio_classify_req(struct fuse_conn *fc, struct fuse_req *req)
>   	case FUSE_FLUSH:
>   	case FUSE_FALLOCATE:
>   		break;
> -	case FUSE_SETATTR:
> +	case FUSE_SETATTR: {
> +		struct fuse_setattr_in const *inarg = req->in.args[0].value;
> +
>   		if (unlikely(!fi || !fi->private))
>   			goto fail;
> -
> +		if (!(inarg->valid & FATTR_SIZE))
> +			return 1;
> +		if (lk)
> +			spin_unlock(&fc->lock);
>   		pcs_kio_setattr_handle(fi, req);
> +		if (lk)
> +			spin_lock(&fc->lock);
>   		return 1;
> +	}
>   	case FUSE_IOCTL: {
>   		struct fuse_ioctl_in const *inarg = req->in.args[0].value;
>   
> @@ -1119,14 +1124,18 @@ static int kpcs_req_send(struct fuse_conn* fc, struct fuse_req *req, bool bg, bo
>   
>   	TRACE(" Enter req:%p op:%d end:%p bg:%d lk:%d\n", req, req->in.h.opcode, req->end, bg, lk);
>   
> -	ret = pcs_kio_classify_req(fc, req);
> +	ret = pcs_kio_classify_req(fc, req, lk);
>   	if (ret) {
>   		if (ret < 0) {
>   			if (!bg)
>   				atomic_inc(&req->count);
>   			__clear_bit(FR_PENDING, &req->flags);
>   			req->out.h.error = ret;
> +			if (lk)
> +				spin_unlock(&fc->lock);
>   			request_end(fc, req);
> +			if (lk)
> +				spin_lock(&fc->lock);
>   			return 0;
>   		}
>   		return 1;
>