| Message ID | 153910100819.28825.6366413700115409017.stgit@localhost.localdomain |
|---|---|
| State | New |
| Series | "fuse kio: Fix deadlock in kpcs_req_send()" |
| Headers | show |
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;
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@virtuozzo.com> Reviewed-by: Pavel Butsykin <pbutsykin@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; >
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@virtuozzo.com> --- fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)