Skip to content

Commit 1a5854e

Browse files
committed
nvmet: fix a possible leak when destroy a ctrl during qp establishment
jira LE-1907 cve CVE-2024-42152 Rebuild_History Non-Buildable kernel-5.14.0-427.33.1.el9_4 commit-author Sagi Grimberg <[email protected]> commit c758b77 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 <[email protected]> Signed-off-by: Sagi Grimberg <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Signed-off-by: Keith Busch <[email protected]> (cherry picked from commit c758b77) Signed-off-by: Jonathan Maple <[email protected]>
1 parent f47a5a0 commit 1a5854e

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
@@ -803,6 +803,15 @@ void nvmet_sq_destroy(struct nvmet_sq *sq)
803803
percpu_ref_exit(&sq->ref);
804804
nvmet_auth_sq_free(sq);
805805

806+
/*
807+
* we must reference the ctrl again after waiting for inflight IO
808+
* to complete. Because admin connect may have sneaked in after we
809+
* store sq->ctrl locally, but before we killed the percpu_ref. the
810+
* admin connect allocates and assigns sq->ctrl, which now needs a
811+
* final ref put, as this ctrl is going away.
812+
*/
813+
ctrl = sq->ctrl;
814+
806815
if (ctrl) {
807816
/*
808817
* The teardown flow may take some time, and the host may not

0 commit comments

Comments
 (0)