[PATCH] nvmet: cache passthru reference in __nvmet_req_complete()

[Nilay Shroff]

__nvmet_req_complete() releases the passthru reference held while
processing a passthru request. However, it currently does so after
calling ops->queue_response().

Once ops->queue_response() is called, the transport may complete and
recycle the request on another CPU. If the request is reused, its
req->p.ref_held field may be reset to false before
__nvmet_req_complete() checks it in nvmet_put_passthru_ref().
Consequently, the passthru reference may not be released, leaving
wait_for_completion(&passthru->disable_done) in
__nvmet_passthru_ctrl_disable() blocked indefinitely.

Fix this by caching the passthru pointer at the beginning of
__nvmet_req_complete(), while the request is still valid, and use
the cached pointer to release the reference after queuing the
response. This also changes nvmet_put_passthru_ref() to accept
the passthru pointer directly instead of accessing it through
the request.

Reported-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
Closes: https://lore.kernel.org/all/auhpmq499RVE9YpC@shinmob/
Fixes: 5e12d81874a1 ("nvmet: fix use-after-free in passthru I/O hotpath")
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
---
 drivers/nvme/target/core.c  |  4 +++-
 drivers/nvme/target/nvmet.h | 24 +++++++++++++++++-------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index eadab017c70b..b8bcb5bf6be6 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -806,6 +806,7 @@ static void __nvmet_req_complete(struct nvmet_req *req, u16 status)
 {
 	struct nvmet_ns *ns = req->ns;
 	struct nvmet_pr_per_ctrl_ref *pc_ref = req->pc_ref;
+	struct nvmet_passthru *passthru = nvmet_passthru_ref_held(req);
 
 	if (!req->sq->sqhd_disabled)
 		nvmet_update_sq_head(req);
@@ -824,7 +825,8 @@ static void __nvmet_req_complete(struct nvmet_req *req, u16 status)
 	if (ns)
 		nvmet_put_namespace(ns);
 
-	nvmet_put_passthru_ref(req);
+	if (passthru)
+		nvmet_put_passthru_ref(passthru);
 }
 
 void nvmet_req_complete(struct nvmet_req *req, u16 status)
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 3cd58e813aa7..2c2460ef5e2b 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -832,6 +832,15 @@ static inline struct nvmet_passthru *nvmet_subsys_passthru(
 	return subsys->passthru;
 }
 
+static inline struct nvmet_passthru *nvmet_passthru_ref_held(
+		struct nvmet_req *req)
+{
+	if (req->p.ref_held)
+		return nvmet_req_subsys(req)->passthru;
+
+	return NULL;
+}
+
 static inline bool nvmet_get_passthru_ref(struct nvmet_req *req)
 {
 	if (!percpu_ref_tryget_live(&nvmet_req_subsys(req)->passthru->ref))
@@ -841,13 +850,9 @@ static inline bool nvmet_get_passthru_ref(struct nvmet_req *req)
 	return true;
 }
 
-static inline void nvmet_put_passthru_ref(struct nvmet_req *req)
+static inline void nvmet_put_passthru_ref(struct nvmet_passthru *passthru)
 {
-	if (!req->p.ref_held)
-		return;
-
-	req->p.ref_held = false;
-	percpu_ref_put(&nvmet_req_subsys(req)->passthru->ref);
+	percpu_ref_put(&passthru->ref);
 }
 #else /* CONFIG_NVME_TARGET_PASSTHRU */
 static inline void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys)
@@ -873,11 +878,16 @@ static inline struct nvmet_passthru *nvmet_subsys_passthru(
 {
 	return NULL;
 }
+static inline struct nvmet_passthru *nvmet_passthru_ref_held(
+		struct nvmet_req *req)
+{
+	return NULL;
+}
 static inline bool nvmet_get_passthru_ref(struct nvmet_req *req)
 {
 	return false;
 }
-static inline void nvmet_put_passthru_ref(struct nvmet_req *req)
+static inline void nvmet_put_passthru_ref(struct nvmet_passthru *passthru)
 {
 }
 #endif /* CONFIG_NVME_TARGET_PASSTHRU */

base-commit: c64d9b2c1f4fd1d8fa52dab37b48428a99c4d58d
-- 
2.53.0