[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