| From ae6f046255fbb400743395eb76f8f65af2063901 Mon Sep 17 00:00:00 2001 |
| From: Israel Rukshin <israelr@mellanox.com> |
| Date: Tue, 4 Feb 2020 14:38:10 +0200 |
| Subject: [PATCH] nvmet: Fix controller use after free |
| |
| commit 1a3f540d63152b8db0a12de508bfa03776217d83 upstream. |
| |
| After nvmet_install_queue() sets sq->ctrl calling to nvmet_sq_destroy() |
| reduces the controller refcount. In case nvmet_install_queue() fails, |
| calling to nvmet_ctrl_put() is done twice (at nvmet_sq_destroy and |
| nvmet_execute_io_connect/nvmet_execute_admin_connect) instead of once for |
| the queue which leads to use after free of the controller. Fix this by set |
| NULL at sq->ctrl in case of a failure at nvmet_install_queue(). |
| |
| The bug leads to the following Call Trace: |
| |
| [65857.994862] refcount_t: underflow; use-after-free. |
| [65858.108304] Workqueue: events nvmet_rdma_release_queue_work [nvmet_rdma] |
| [65858.115557] RIP: 0010:refcount_warn_saturate+0xe5/0xf0 |
| [65858.208141] Call Trace: |
| [65858.211203] nvmet_sq_destroy+0xe1/0xf0 [nvmet] |
| [65858.216383] nvmet_rdma_release_queue_work+0x37/0xf0 [nvmet_rdma] |
| [65858.223117] process_one_work+0x167/0x370 |
| [65858.227776] worker_thread+0x49/0x3e0 |
| [65858.232089] kthread+0xf5/0x130 |
| [65858.235895] ? max_active_store+0x80/0x80 |
| [65858.240504] ? kthread_bind+0x10/0x10 |
| [65858.244832] ret_from_fork+0x1f/0x30 |
| [65858.249074] ---[ end trace f82d59250b54beb7 ]--- |
| |
| Fixes: bb1cc74790eb ("nvmet: implement valid sqhd values in completions") |
| Fixes: 1672ddb8d691 ("nvmet: Add install_queue callout") |
| Signed-off-by: Israel Rukshin <israelr@mellanox.com> |
| Reviewed-by: Max Gurtovoy <maxg@mellanox.com> |
| Reviewed-by: Christoph Hellwig <hch@lst.de> |
| Signed-off-by: Keith Busch <kbusch@kernel.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c |
| index 1c119eef3e3b..129f11a8a522 100644 |
| --- a/drivers/nvme/target/fabrics-cmd.c |
| +++ b/drivers/nvme/target/fabrics-cmd.c |
| @@ -105,6 +105,7 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req) |
| u16 qid = le16_to_cpu(c->qid); |
| u16 sqsize = le16_to_cpu(c->sqsize); |
| struct nvmet_ctrl *old; |
| + u16 ret; |
| |
| old = cmpxchg(&req->sq->ctrl, NULL, ctrl); |
| if (old) { |
| @@ -115,7 +116,8 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req) |
| if (!sqsize) { |
| pr_warn("queue size zero!\n"); |
| req->error_loc = offsetof(struct nvmf_connect_command, sqsize); |
| - return NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR; |
| + ret = NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR; |
| + goto err; |
| } |
| |
| /* note: convert queue size from 0's-based value to 1's-based value */ |
| @@ -128,16 +130,19 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req) |
| } |
| |
| if (ctrl->ops->install_queue) { |
| - u16 ret = ctrl->ops->install_queue(req->sq); |
| - |
| + ret = ctrl->ops->install_queue(req->sq); |
| if (ret) { |
| pr_err("failed to install queue %d cntlid %d ret %x\n", |
| qid, ctrl->cntlid, ret); |
| - return ret; |
| + goto err; |
| } |
| } |
| |
| return 0; |
| + |
| +err: |
| + req->sq->ctrl = NULL; |
| + return ret; |
| } |
| |
| static void nvmet_execute_admin_connect(struct nvmet_req *req) |
| -- |
| 2.7.4 |
| |