[bug report] nvmet: implement unique discovery NQN
Dan Carpenter
dan.carpenter at linaro.org
Wed May 8 00:39:32 PDT 2024
Hello Hannes Reinecke,
Commit 95409e277d83 ("nvmet: implement unique discovery NQN") from
Apr 3, 2024 (linux-next), leads to the following Smatch static
checker warning:
drivers/nvme/target/configfs.c:2206 nvmet_root_discovery_nqn_store()
warn: 'nvmet_disc_subsys->subsysnqn' sometimes too small '224' size = 256
drivers/nvme/target/configfs.c
2185 static ssize_t nvmet_root_discovery_nqn_store(struct config_item *item,
2186 const char *page, size_t count)
2187 {
2188 struct list_head *entry;
2189 size_t len;
2190
2191 len = strcspn(page, "\n");
2192 if (!len || len > NVMF_NQN_FIELD_LEN - 1)
^^^^^^^^^^^^^^^^^^^^^^
There is some kind of mix up between these.
/* NQN names in commands fields specified one size */
#define NVMF_NQN_FIELD_LEN 256
/* However the max length of a qualified name is another size */
#define NVMF_NQN_SIZE 223
2193 return -EINVAL;
2194
2195 down_write(&nvmet_config_sem);
2196 list_for_each(entry, &nvmet_subsystems_group.cg_children) {
2197 struct config_item *item =
2198 container_of(entry, struct config_item, ci_entry);
2199
2200 if (!strncmp(config_item_name(item), page, len)) {
2201 pr_err("duplicate NQN %s\n", config_item_name(item));
2202 up_write(&nvmet_config_sem);
2203 return -EINVAL;
2204 }
2205 }
--> 2206 memset(nvmet_disc_subsys->subsysnqn, 0, NVMF_NQN_FIELD_LEN);
^^^^^^^^^^^^^^^^^^
2207 memcpy(nvmet_disc_subsys->subsysnqn, page, len);
^^^
Which leads to memory corruption. The nvmet_disc_subsys->subsysnqn
struct member is allocated in nvmet_subsys_alloc() with a MAXIMUM of
224 bytes depending on the input string.
subsys->subsysnqn = kstrndup(subsysnqn, NVMF_NQN_SIZE, GFP_KERNEL);
There is also a harmless off by one. kstrndup() is allocating up to
NVMF_NQN_SIZE + 1, but at the top of this function we're subtracting 1.
It should be allocated like:
subsys->subsysnqn = kzalloc(subsysnqn, NVMF_NQN_SIZE or NVMF_NQN_FIELD_LEN + 1, GFP_KERNEL);
if (!subsys->subsysnqn)
return -ENOMEM or whatever;
strscpy(subsys->subsysnqn, subsysnqn);
2208 up_write(&nvmet_config_sem);
2209
2210 return len;
2211 }
regards,
dan carpenter
More information about the Linux-nvme
mailing list