Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Commit c758b77

Browse files
sagigrimbergkeithbusch
authored andcommitted
nvmet: fix a possible leak when destroy a ctrl during qp establishment
In nvmet_sq_destroy we capture sq->ctrl early and if it is non-NULL we know that a ctrl was allocated (in the admin connect request handler) and we need to release pending AERs, clear ctrl->sqs and sq->ctrl (for nvme-loop primarily), and drop the final reference on the ctrl. However, a small window is possible where nvmet_sq_destroy starts (as a result of the client giving up and disconnecting) concurrently with the nvme admin connect cmd (which may be in an early stage). But *before* kill_and_confirm of sq->ref (i.e. the admin connect managed to get an sq live reference). In this case, sq->ctrl was allocated however after it was captured in a local variable in nvmet_sq_destroy. This prevented the final reference drop on the ctrl. Solve this by re-capturing the sq->ctrl after all inflight request has completed, where for sure sq->ctrl reference is final, and move forward based on that. This issue was observed in an environment with many hosts connecting multiple ctrls simoutanuosly, creating a delay in allocating a ctrl leading up to this race window. Reported-by: Alex Turin <alex@vastdata.com> Signed-off-by: Sagi Grimberg <sagi@grimberg.me> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Keith Busch <kbusch@kernel.org>
1 parent be647e2 commit c758b77

File tree

1 file changed

+9
-0
lines changed

1 file changed

+9
-0
lines changed

drivers/nvme/target/core.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,15 @@ void nvmet_sq_destroy(struct nvmet_sq *sq)
818818
percpu_ref_exit(&sq->ref);
819819
nvmet_auth_sq_free(sq);
820820

821+
/*
822+
* we must reference the ctrl again after waiting for inflight IO
823+
* to complete. Because admin connect may have sneaked in after we
824+
* store sq->ctrl locally, but before we killed the percpu_ref. the
825+
* admin connect allocates and assigns sq->ctrl, which now needs a
826+
* final ref put, as this ctrl is going away.
827+
*/
828+
ctrl = sq->ctrl;
829+
821830
if (ctrl) {
822831
/*
823832
* The teardown flow may take some time, and the host may not

0 commit comments

Comments
 (0)