[PATCH 1/4] nvmet: forbid changing ctrl ID attributes for discovered subsystems

[Max Gurtovoy]

Controller identifiers are dynamically assigned when an NVMe host
connects to a target. The minimum and maximum allowed controller ID
values for a subsystem are configurable via configfs. Do not allow
changes to these attributes after a subsystem has already been
discovered to prevent invalid configuration.

Signed-off-by: Max Gurtovoy <mgurtovoy at nvidia.com>
---
 drivers/nvme/target/admin-cmd.c |  6 ----
 drivers/nvme/target/configfs.c  | 63 ++++++++++++++++++++++++---------
 drivers/nvme/target/core.c      |  6 ++++
 3 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 3e378153a781..72c741a95ac8 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -654,12 +654,6 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 	u32 cmd_capsule_size, ctratt;
 	u16 status = 0;
 
-	if (!subsys->subsys_discovered) {
-		mutex_lock(&subsys->lock);
-		subsys->subsys_discovered = true;
-		mutex_unlock(&subsys->lock);
-	}
-
 	id = kzalloc(sizeof(*id), GFP_KERNEL);
 	if (!id) {
 		status = NVME_SC_INTERNAL;
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e44ef69dffc2..979eb184756a 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1348,10 +1348,30 @@ static ssize_t nvmet_subsys_attr_cntlid_min_show(struct config_item *item,
 	return snprintf(page, PAGE_SIZE, "%u\n", to_subsys(item)->cntlid_min);
 }
 
+static ssize_t
+nvmet_subsys_attr_cntlid_min_store_locked(struct nvmet_subsys *subsys,
+		u16 cntlid_min, size_t cnt)
+{
+
+	if (subsys->subsys_discovered) {
+		pr_err("Can't set minimal cntlid. %u is already assigned\n",
+		       subsys->cntlid_min);
+		return -EINVAL;
+	}
+
+	if (cntlid_min > subsys->cntlid_max)
+		return -EINVAL;
+
+	subsys->cntlid_min = cntlid_min;
+	return cnt;
+}
+
 static ssize_t nvmet_subsys_attr_cntlid_min_store(struct config_item *item,
 						  const char *page, size_t cnt)
 {
+	struct nvmet_subsys *subsys = to_subsys(item);
 	u16 cntlid_min;
+	ssize_t ret;
 
 	if (sscanf(page, "%hu\n", &cntlid_min) != 1)
 		return -EINVAL;
@@ -1360,15 +1380,11 @@ static ssize_t nvmet_subsys_attr_cntlid_min_store(struct config_item *item,
 		return -EINVAL;
 
 	down_write(&nvmet_config_sem);
-	if (cntlid_min > to_subsys(item)->cntlid_max)
-		goto out_unlock;
-	to_subsys(item)->cntlid_min = cntlid_min;
-	up_write(&nvmet_config_sem);
-	return cnt;
-
-out_unlock:
+	mutex_lock(&subsys->lock);
+	ret = nvmet_subsys_attr_cntlid_min_store_locked(subsys, cntlid_min, cnt);
+	mutex_unlock(&subsys->lock);
 	up_write(&nvmet_config_sem);
-	return -EINVAL;
+	return ret;
 }
 CONFIGFS_ATTR(nvmet_subsys_, attr_cntlid_min);
 
@@ -1378,10 +1394,29 @@ static ssize_t nvmet_subsys_attr_cntlid_max_show(struct config_item *item,
 	return snprintf(page, PAGE_SIZE, "%u\n", to_subsys(item)->cntlid_max);
 }
 
+static ssize_t
+nvmet_subsys_attr_cntlid_max_store_locked(struct nvmet_subsys *subsys,
+		u16 cntlid_max, size_t cnt)
+{
+
+	if (subsys->subsys_discovered) {
+		pr_err("Can't set maximal cntlid. %u is already assigned\n",
+		       subsys->cntlid_max);
+		return -EINVAL;
+	}
+
+	if (cntlid_max < subsys->cntlid_min)
+		return -EINVAL;
+	subsys->cntlid_max = cntlid_max;
+	return cnt;
+}
+
 static ssize_t nvmet_subsys_attr_cntlid_max_store(struct config_item *item,
 						  const char *page, size_t cnt)
 {
+	struct nvmet_subsys *subsys = to_subsys(item);
 	u16 cntlid_max;
+	ssize_t ret;
 
 	if (sscanf(page, "%hu\n", &cntlid_max) != 1)
 		return -EINVAL;
@@ -1390,15 +1425,11 @@ static ssize_t nvmet_subsys_attr_cntlid_max_store(struct config_item *item,
 		return -EINVAL;
 
 	down_write(&nvmet_config_sem);
-	if (cntlid_max < to_subsys(item)->cntlid_min)
-		goto out_unlock;
-	to_subsys(item)->cntlid_max = cntlid_max;
-	up_write(&nvmet_config_sem);
-	return cnt;
-
-out_unlock:
+	mutex_lock(&subsys->lock);
+	ret = nvmet_subsys_attr_cntlid_max_store_locked(subsys, cntlid_max, cnt);
+	mutex_unlock(&subsys->lock);
 	up_write(&nvmet_config_sem);
-	return -EINVAL;
+	return ret;
 }
 CONFIGFS_ATTR(nvmet_subsys_, attr_cntlid_max);
 
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 099a05409ac5..20e7b3d6a810 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1667,6 +1667,12 @@ struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args)
 	if (!ctrl->changed_ns_list)
 		goto out_free_ctrl;
 
+	if (!subsys->subsys_discovered) {
+		mutex_lock(&subsys->lock);
+		subsys->subsys_discovered = true;
+		mutex_unlock(&subsys->lock);
+	}
+
 	ctrl->sqs = kcalloc(subsys->max_qid + 1,
 			sizeof(struct nvmet_sq *),
 			GFP_KERNEL);
-- 
2.18.1