[PATCH v2] nvmet: verify the hostid when looking up a controller
Yifei Chu <[email protected]>
| Newsgroups | org.infradead.lists.linux-nvme |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
Assisted-by: abaci:qwen3.8-max
Signed-off-by: Yifei Chu <[email protected]>
---
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/[email protected]/
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