[PATCH 1/1] nvmet: allow setting model_number once
Max Gurtovoy
mgurtovoy at nvidia.com
Wed Feb 17 13:11:53 EST 2021
In case we have already established connection to nvmf target, it
shouldn't be allowed to change the model_number. E.g. if someone will
identify ctrl and get model_number of "my_model" later on will change
the model_numbel via configfs to "my_new_model" this will break the NVM
specification for "Get Log Page – Persistent Event Log" that refers to
Model Number as: "This field contains the same value as reported in the
Model Number field of the Identify Controller data structure, bytes
63:24."
Although it doesn't mentioned explicitly that this field can't be
changed, we can assume it.
So allow setting this field only once: using configfs or in the first
identify ctrl operation.
Signed-off-by: Max Gurtovoy <mgurtovoy at nvidia.com>
---
drivers/nvme/target/admin-cmd.c | 33 +++++++++++++------
drivers/nvme/target/configfs.c | 57 ++++++++++++++++++---------------
drivers/nvme/target/core.c | 2 +-
drivers/nvme/target/nvmet.h | 7 +---
4 files changed, 57 insertions(+), 42 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index dc1ea468b182..6a3341b2091f 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -319,18 +319,25 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR);
}
-static void nvmet_id_set_model_number(struct nvme_id_ctrl *id,
+static int nvmet_id_set_model_number(struct nvme_id_ctrl *id,
struct nvmet_subsys *subsys)
{
- const char *model = NVMET_DEFAULT_CTRL_MODEL;
- struct nvmet_subsys_model *subsys_model;
+ int ret = 0;
- rcu_read_lock();
- subsys_model = rcu_dereference(subsys->model);
- if (subsys_model)
- model = subsys_model->number;
- memcpy_and_pad(id->mn, sizeof(id->mn), model, strlen(model), ' ');
- rcu_read_unlock();
+ mutex_lock(&subsys->lock);
+ if (!subsys->model_number) {
+ subsys->model_number = kstrdup(NVMET_DEFAULT_CTRL_MODEL,
+ GFP_KERNEL);
+ if (!subsys->model_number) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+ }
+ memcpy_and_pad(id->mn, sizeof(id->mn), subsys->model_number,
+ strlen(subsys->model_number), ' ');
+out_unlock:
+ mutex_unlock(&subsys->lock);
+ return ret;
}
static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
@@ -339,6 +346,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
struct nvme_id_ctrl *id;
u32 cmd_capsule_size;
u16 status = 0;
+ int ret;
id = kzalloc(sizeof(*id), GFP_KERNEL);
if (!id) {
@@ -353,7 +361,11 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
memset(id->sn, ' ', sizeof(id->sn));
bin2hex(id->sn, &ctrl->subsys->serial,
min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
- nvmet_id_set_model_number(id, ctrl->subsys);
+ ret = nvmet_id_set_model_number(id, ctrl->subsys);
+ if (ret) {
+ status = NVME_SC_INTERNAL;
+ goto out_free;
+ }
memcpy_and_pad(id->fr, sizeof(id->fr),
UTS_RELEASE, strlen(UTS_RELEASE), ' ');
@@ -461,6 +473,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+out_free:
kfree(id);
out:
nvmet_req_complete(req, status);
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index c61ffd767062..ad1adcf30ee5 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1120,16 +1120,14 @@ static ssize_t nvmet_subsys_attr_model_show(struct config_item *item,
char *page)
{
struct nvmet_subsys *subsys = to_subsys(item);
- struct nvmet_subsys_model *subsys_model;
- char *model = NVMET_DEFAULT_CTRL_MODEL;
+ char *model = "";
int ret;
- rcu_read_lock();
- subsys_model = rcu_dereference(subsys->model);
- if (subsys_model)
- model = subsys_model->number;
+ mutex_lock(&subsys->lock);
+ if (subsys->model_number)
+ model = subsys->model_number;
ret = snprintf(page, PAGE_SIZE, "%s\n", model);
- rcu_read_unlock();
+ mutex_unlock(&subsys->lock);
return ret;
}
@@ -1144,40 +1142,49 @@ static ssize_t nvmet_subsys_attr_model_store(struct config_item *item,
const char *page, size_t count)
{
struct nvmet_subsys *subsys = to_subsys(item);
- struct nvmet_subsys_model *new_model;
char *new_model_number;
int pos = 0, len;
+ down_write(&nvmet_config_sem);
+ mutex_lock(&subsys->lock);
+ if (subsys->model_number) {
+ pr_err("Can't set model number. %s is already assigned\n",
+ subsys->model_number);
+ count = -EINVAL;
+ goto out_unlock;
+ }
+
len = strcspn(page, "\n");
- if (!len)
- return -EINVAL;
+ if (!len) {
+ count = -EINVAL;
+ goto out_unlock;
+ }
for (pos = 0; pos < len; pos++) {
- if (!nvmet_is_ascii(page[pos]))
- return -EINVAL;
+ if (!nvmet_is_ascii(page[pos])) {
+ count = -EINVAL;
+ goto out_unlock;
+ }
}
new_model_number = kmemdup_nul(page, len, GFP_KERNEL);
- if (!new_model_number)
- return -ENOMEM;
+ if (!new_model_number) {
+ count = -ENOMEM;
+ goto out_unlock;
+ }
- new_model = kzalloc(sizeof(*new_model) + len + 1, GFP_KERNEL);
- if (!new_model) {
+ subsys->model_number = kstrdup(new_model_number, GFP_KERNEL);
+ if (!subsys->model_number) {
+ count = -ENOMEM;
kfree(new_model_number);
- return -ENOMEM;
+ goto out_unlock;
}
- memcpy(new_model->number, new_model_number, len);
- down_write(&nvmet_config_sem);
- mutex_lock(&subsys->lock);
- new_model = rcu_replace_pointer(subsys->model, new_model,
- mutex_is_locked(&subsys->lock));
+ kfree(new_model_number);
+out_unlock:
mutex_unlock(&subsys->lock);
up_write(&nvmet_config_sem);
- kfree_rcu(new_model, rcuhead);
- kfree(new_model_number);
-
return count;
}
CONFIGFS_ATTR(nvmet_subsys_, attr_model);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 8ce4d59cc9e7..c7af907912f2 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1521,7 +1521,7 @@ static void nvmet_subsys_free(struct kref *ref)
nvmet_passthru_subsys_free(subsys);
kfree(subsys->subsysnqn);
- kfree_rcu(subsys->model, rcuhead);
+ kfree(subsys->model_number);
kfree(subsys);
}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 592763732065..aac741bf378c 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -208,11 +208,6 @@ struct nvmet_ctrl {
bool pi_support;
};
-struct nvmet_subsys_model {
- struct rcu_head rcuhead;
- char number[];
-};
-
struct nvmet_subsys {
enum nvme_subsys_type type;
@@ -242,7 +237,7 @@ struct nvmet_subsys {
struct config_group namespaces_group;
struct config_group allowed_hosts_group;
- struct nvmet_subsys_model __rcu *model;
+ char *model_number;
#ifdef CONFIG_NVME_TARGET_PASSTHRU
struct nvme_ctrl *passthru_ctrl;
--
2.25.4
More information about the Linux-nvme
mailing list