[PATCH 2/2] nvmet: fix use-after-free error in passthru I/O hotpath

[Nilay Shroff <nilay@linux.ibm.com>]

Concurrently disabling a passthru controller while passthru I/Os are
in flight can potentially result in a use-after-free. Introduce a
percpu refcount, an atomic flag, and an nvmet request flag to track
reference held by passthru I/Os and protect the passthru controller
lifetime.

When enabling the passthru controller, initialize the percpu refcount
and set the enabled flag. Each passthru I/O acquires a reference before
entering the passthru hot path and sets the nvmet request flag to record
that the reference is held. The reference is released when the I/O
completes and the request flag is set.

When disabling the passthru controller, clear the enabled flag, kill the
percpu refcount, and wait for all in-flight I/Os to release their
references before releasing the passthru controller. Concurrent disable
attempts are serialized by the atomic enabled flag: only the first
disable attempt observes the flag set and proceeds, while subsequent
attempts bail out.

Once disabling starts, new I/Os fail to acquire a live reference and
therefore cannot enter the passthru hot path.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/target/core.c     |  4 +++
 drivers/nvme/target/nvmet.h    | 23 ++++++++++++++
 drivers/nvme/target/passthru.c | 58 ++++++++++++++++++++++++++++------
 3 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 9ab07dbe8cbe..0870977eccf6 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -819,6 +819,9 @@ static void __nvmet_req_complete(struct nvmet_req *req, u16 status)
 		nvmet_pr_put_ns_pc_ref(pc_ref);
 	if (ns)
 		nvmet_put_namespace(ns);
+
+	if (req->p.ref_held)
+		nvmet_put_passthru_ref(req);
 }
 
 void nvmet_req_complete(struct nvmet_req *req, u16 status)
@@ -1195,6 +1198,7 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_sq *sq,
 	req->error_loc = NVMET_NO_ERROR_LOC;
 	req->error_slba = 0;
 	req->pc_ref = NULL;
+	req->p.ref_held = false;
 
 	/* no support for fused commands yet */
 	if (unlikely(flags & (NVME_CMD_FUSE_FIRST | NVME_CMD_FUSE_SECOND))) {
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8f5dccee7d26..2953102a90bd 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -320,6 +320,11 @@ struct nvmet_ctrl {
 };
 
 struct nvmet_passthru {
+	struct percpu_ref	ref;
+	struct completion	disable_done;
+#define NVMET_PASSTHRU_ENABLED	0
+	unsigned long		flags;
+
 	struct nvme_ctrl	*ctrl;
 	char			*ctrl_path;
 	struct config_group	group;
@@ -478,6 +483,7 @@ struct nvmet_req {
 			struct request		*rq;
 			struct work_struct      work;
 			bool			use_workqueue;
+			bool			ref_held;
 		} p;
 #ifdef CONFIG_BLK_DEV_ZONED
 		struct {
@@ -799,6 +805,16 @@ static inline bool nvmet_is_passthru_subsys(struct nvmet_subsys *subsys)
 {
 	return subsys->passthru.ctrl;
 }
+
+static inline bool nvmet_get_passthru_ref(struct nvmet_req *req)
+{
+	return percpu_ref_tryget_live(&nvmet_req_subsys(req)->passthru.ref);
+}
+
+static inline void nvmet_put_passthru_ref(struct nvmet_req *req)
+{
+	percpu_ref_put(&nvmet_req_subsys(req)->passthru.ref);
+}
 #else /* CONFIG_NVME_TARGET_PASSTHRU */
 static inline void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys)
 {
@@ -818,6 +834,13 @@ static inline bool nvmet_is_passthru_subsys(struct nvmet_subsys *subsys)
 {
 	return NULL;
 }
+static inline bool nvmet_get_passthru_ref(struct nvmet_req *req)
+{
+	return NULL;
+}
+static inline void nvmet_put_passthru_ref(struct nvmet_req *req)
+{
+}
 #endif /* CONFIG_NVME_TARGET_PASSTHRU */
 
 static inline bool nvmet_is_passthru_req(struct nvmet_req *req)
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 81ac220da8ba..c18b1dda2a18 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -305,9 +305,9 @@ static int nvmet_passthru_map_sg(struct nvmet_req *req, struct request *rq)
 
 static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
 {
-	struct nvmet_passthru *passthru = &nvmet_req_subsys(req)->passthru;
-	struct nvme_ctrl *ctrl = passthru->ctrl;
-	struct request_queue *q = ctrl->admin_q;
+	struct nvmet_passthru *passthru;
+	struct nvme_ctrl *ctrl;
+	struct request_queue *q;
 	struct nvme_ns *ns = NULL;
 	struct request *rq = NULL;
 	unsigned int timeout;
@@ -315,6 +315,16 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
 	u16 status;
 	int ret;
 
+	req->p.ref_held = nvmet_get_passthru_ref(req);
+	if (!req->p.ref_held) {
+		status = NVME_SC_INTERNAL | NVME_STATUS_DNR;
+		goto out;
+	}
+
+	passthru = &nvmet_req_subsys(req)->passthru;
+	ctrl = passthru->ctrl;
+	q = ctrl->admin_q;
+
 	if (likely(req->sq->qid != 0)) {
 		u32 nsid = le32_to_cpu(req->cmd->common.nsid);
 
@@ -387,11 +397,18 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
  */
 static void nvmet_passthru_set_host_behaviour(struct nvmet_req *req)
 {
-	struct nvme_ctrl *ctrl = nvmet_req_subsys(req)->passthru.ctrl;
+	struct nvme_ctrl *ctrl;
 	struct nvme_feat_host_behavior *host;
 	u16 status = NVME_SC_INTERNAL;
 	int ret;
 
+	req->p.ref_held = nvmet_get_passthru_ref(req);
+	if (!req->p.ref_held) {
+		status |= NVME_STATUS_DNR;
+		goto out_complete_req;
+	}
+	ctrl = nvmet_req_subsys(req)->passthru.ctrl;
+
 	host = kzalloc(sizeof(*host) * 2, GFP_KERNEL);
 	if (!host)
 		goto out_complete_req;
@@ -585,6 +602,14 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
 	}
 }
 
+static void nvmet_release_passthru_ctrl(struct percpu_ref *ref)
+{
+	struct nvmet_passthru *passthru = container_of(ref,
+				struct nvmet_passthru, ref);
+
+	complete(&passthru->disable_done);
+}
+
 int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 {
 	struct nvmet_passthru *passthru = &subsys->passthru;
@@ -628,9 +653,15 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 	if (old)
 		goto out_put_file;
 
+	ret = percpu_ref_init(&passthru->ref, nvmet_release_passthru_ctrl,
+			0, GFP_KERNEL);
+	if (ret) {
+		xa_erase(&passthru_subsystems, ctrl->instance);
+		goto out_put_file;
+	}
+	init_completion(&passthru->disable_done);
 	passthru->ctrl = ctrl;
 	subsys->ver = ctrl->vs;
-
 	if (subsys->ver < NVME_VS(1, 2, 1)) {
 		pr_warn("nvme controller version is too old: %llu.%llu.%llu, advertising 1.2.1\n",
 			NVME_MAJOR(subsys->ver), NVME_MINOR(subsys->ver),
@@ -639,6 +670,7 @@ int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys)
 	}
 	nvme_get_ctrl(ctrl);
 	__module_get(passthru->ctrl->ops->module);
+	set_bit(NVMET_PASSTHRU_ENABLED, &passthru->flags);
 	ret = 0;
 
 out_put_file:
@@ -652,11 +684,17 @@ static void __nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys)
 {
 	struct nvmet_passthru *passthru = &subsys->passthru;
 
-	if (passthru->ctrl) {
-		xa_erase(&passthru_subsystems, passthru->ctrl->instance);
-		module_put(passthru->ctrl->ops->module);
-		nvme_put_ctrl(passthru->ctrl);
-	}
+	if (!test_and_clear_bit(NVMET_PASSTHRU_ENABLED, &passthru->flags))
+		return;
+
+	percpu_ref_kill(&passthru->ref);
+	wait_for_completion(&passthru->disable_done);
+	percpu_ref_exit(&passthru->ref);
+
+	xa_erase(&passthru_subsystems, passthru->ctrl->instance);
+	module_put(passthru->ctrl->ops->module);
+	nvme_put_ctrl(passthru->ctrl);
+
 	passthru->ctrl = NULL;
 	subsys->ver = NVMET_DEFAULT_VS;
 }
-- 
2.53.0