[PATCH v2] nvmet: verify the hostid when looking up a controller

[PATCH v2] nvmet: verify the hostid when looking up a controller

[Yifei Chu]

nvmet_ctrl_find_get() matches controllers by cntlid and hostnqn only.
The connect data also carries the connecting host's hostid, but it is
never compared against the hostid the controller was created with, so
the lookup can return a controller whose recorded hostid differs.

Controller IDs are allocated sequentially and easy to guess, and
nothing prevents two hosts from using the same hostnqn (misconfigured
clones, or a reinstalled host whose controller still exists). Such a
host currently attaches its queues to the other host's controller and
shares its controller state with it. Also require the hostid from the
connect data to match the controller's hostid so that the lookup is
bound to the identity the controller was created with.

Controllers that were created without a hostid keep the old behaviour.

Fixes: a07b4970f464 ("nvmet: add a generic NVMe target")
Reported-by: Abaci <abaci@linux.alibaba.com>
Assisted-by: abaci:qwen3.8-max
Signed-off-by: Yifei Chu <Chuyf26@linux.alibaba.com>
---
v2: reword the changelog as a correctness fix instead of a security
    fix, the hostid is not a secret either (Sagi).
    Previous thread:
    https://lore.kernel.org/all/178707054946.2644927.16076495238405774277@linux.alibaba.com/

 drivers/nvme/target/core.c        | 12 ++++++++++++
 drivers/nvme/target/fabrics-cmd.c |  2 +-
 drivers/nvme/target/nvmet.h       |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index d74c01c..f1eb5cc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1471,6 +1471,7 @@ static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
 
 struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				       const char *hostnqn, u16 cntlid,
+				       const uuid_t *hostid,
 				       struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = NULL;
@@ -1491,6 +1492,17 @@ struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				pr_warn("hostnqn mismatch.\n");
 				continue;
 			}
+			/*
+			 * Also require the hostid from the connect data to
+			 * match the hostid the controller was created with.
+			 * Accept a nil hostid only if the controller was
+			 * created without one.
+			 */
+			if (!uuid_is_null(&ctrl->hostid) &&
+			    !uuid_equal(&ctrl->hostid, hostid)) {
+				pr_warn("hostid mismatch.\n");
+				continue;
+			}
 			if (!kref_get_unless_zero(&ctrl->ref))
 				continue;
 
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index 42d1d18..5e2487d 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -364,7 +364,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 	d->subsysnqn[NVMF_NQN_FIELD_LEN - 1] = '\0';
 	d->hostnqn[NVMF_NQN_FIELD_LEN - 1] = '\0';
 	ctrl = nvmet_ctrl_find_get(d->subsysnqn, d->hostnqn,
-				   le16_to_cpu(d->cntlid), req);
+				   le16_to_cpu(d->cntlid), &d->hostid, req);
 	if (!ctrl) {
 		status = NVME_SC_CONNECT_INVALID_PARAM | NVME_STATUS_DNR;
 		goto out;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index e362d79..52ea4c7 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -623,6 +623,7 @@ struct nvmet_alloc_ctrl_args {
 struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args);
 struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				       const char *hostnqn, u16 cntlid,
+				       const uuid_t *hostid,
 				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
 u16 nvmet_check_ctrl_status(struct nvmet_req *req);
-- 
2.43.5

Re: [PATCH v2] nvmet: verify the hostid when looking up a controller

[Sagi Grimberg]

On 27/08/2026 13:12, Yifei Chu wrote:
> nvmet_ctrl_find_get() matches controllers by cntlid and hostnqn only.
> The connect data also carries the connecting host's hostid, but it is
> never compared against the hostid the controller was created with, so
> the lookup can return a controller whose recorded hostid differs.
>
> Controller IDs are allocated sequentially and easy to guess, and
> nothing prevents two hosts from using the same hostnqn (misconfigured
> clones, or a reinstalled host whose controller still exists). Such a
> host currently attaches its queues to the other host's controller and
> shares its controller state with it. Also

This paragraph is pretty useless IMO. I think you can drop it.

> require the hostid from the
> connect data to match the controller's hostid so that the lookup is
> bound to the identity the controller was created with.

Keep this.

Other than that,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH v3] nvmet: verify the hostid when looking up a controller

[Yifei Chu]

nvmet_ctrl_find_get() matches controllers by cntlid and hostnqn only.
The connect data also carries the connecting host's hostid, but it is
never compared against the hostid the controller was created with, so
the lookup can return a controller whose recorded hostid differs.

Require the hostid from the connect data to match the controller's
hostid so that the lookup is bound to the identity the controller was
created with.

Controllers that were created without a hostid keep the old behaviour.

Fixes: a07b4970f464 ("nvmet: add a generic NVMe target")
Reported-by: Abaci <abaci@linux.alibaba.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Assisted-by: abaci:qwen3.8-max
Signed-off-by: Yifei Chu <Chuyf26@linux.alibaba.com>
---
v3: drop the second paragraph of the changelog (Sagi).
    Collected Sagi's Reviewed-by.

 drivers/nvme/target/core.c        | 12 ++++++++++++
 drivers/nvme/target/fabrics-cmd.c |  2 +-
 drivers/nvme/target/nvmet.h       |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index d74c01c..f1eb5cc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1471,6 +1471,7 @@ static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
 
 struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				       const char *hostnqn, u16 cntlid,
+				       const uuid_t *hostid,
 				       struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = NULL;
@@ -1491,6 +1492,17 @@ struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				pr_warn("hostnqn mismatch.\n");
 				continue;
 			}
+			/*
+			 * Also require the hostid from the connect data to
+			 * match the hostid the controller was created with.
+			 * Accept a nil hostid only if the controller was
+			 * created without one.
+			 */
+			if (!uuid_is_null(&ctrl->hostid) &&
+			    !uuid_equal(&ctrl->hostid, hostid)) {
+				pr_warn("hostid mismatch.\n");
+				continue;
+			}
 			if (!kref_get_unless_zero(&ctrl->ref))
 				continue;
 
diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index 42d1d18..5e2487d 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -364,7 +364,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 	d->subsysnqn[NVMF_NQN_FIELD_LEN - 1] = '\0';
 	d->hostnqn[NVMF_NQN_FIELD_LEN - 1] = '\0';
 	ctrl = nvmet_ctrl_find_get(d->subsysnqn, d->hostnqn,
-				   le16_to_cpu(d->cntlid), req);
+				   le16_to_cpu(d->cntlid), &d->hostid, req);
 	if (!ctrl) {
 		status = NVME_SC_CONNECT_INVALID_PARAM | NVME_STATUS_DNR;
 		goto out;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index e362d79..52ea4c7 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -623,6 +623,7 @@ struct nvmet_alloc_ctrl_args {
 struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args);
 struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				       const char *hostnqn, u16 cntlid,
+				       const uuid_t *hostid,
 				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
 u16 nvmet_check_ctrl_status(struct nvmet_req *req);
-- 
2.43.5

Re: [PATCH v3] nvmet: verify the hostid when looking up a controller

[Christoph Hellwig]

On Mon, Aug 31, 2026 at 10:36:51AM +0800, Yifei Chu wrote:
> nvmet_ctrl_find_get() matches controllers by cntlid and hostnqn only.
> The connect data also carries the connecting host's hostid, but it is
> never compared against the hostid the controller was created with, so
> the lookup can return a controller whose recorded hostid differs.

So?  The hostid does not identify a controller.

Re: [PATCH v3] nvmet: verify the hostid when looking up a controller

[Yifei Chu]

On Wed, Sep 02, 2026 at 04:04:00PM +0200, Christoph Hellwig wrote:
> So?  The hostid does not identify a controller.

Agreed, the controller is identified by cntlid and hostnqn. The
check is not about identifying the controller; it is about the host
identity bound to the controller at creation time:

- The fabrics connect path has a dedicated status for rejecting a
  connect because of the host identity: NVME_SC_CONNECT_INVALID_HOST
  (include/linux/nvme.h). nvmet already returns it from
  nvmet_alloc_ctrl() when the hostnqn is not allowed, but the hostid
  half of the host identity is never compared when io queues attach
  to an existing controller.

- The Linux host fills the same hostid into every connect capsule
  (nvmf_connect_data_prep() copies ctrl->opts->host->id, used by
  both the admin and the io connect path), and it refuses locally to
  pair one hostnqn with a different hostid ("maintain unambiguous
  host identification"). A conforming host therefore can never fail
  this check.

- nvmet consumes ctrl->hostid as the host identity: persistent
  reservation registrants and holders are keyed by it
  (drivers/nvme/target/pr.c). Letting an io connect with a different
  hostid attach its queues to the controller attributes those queues
  to a host the controller was not created for.

Comparing both halves of the host identity in the lookup is the
symmetric counterpart of the existing hostnqn comparison. If the
nvme maintainers disagree, I will drop the patch.

Yifei Chu