Commit 3e0ef1b8 authored by Solganik Alexander's avatar Solganik Alexander Committed by Greg Kroah-Hartman

nvmet: Fix possible infinite loop triggered on hot namespace removal

commit e4fcf07c upstream.

When removing a namespace we delete it from the subsystem namespaces
list with list_del_init which allows us to know if it is enabled or
not.

The problem is that list_del_init initialize the list next and does
not respect the RCU list-traversal we do on the IO path for locating
a namespace. Instead we need to use list_del_rcu which is allowed to
run concurrently with the _rcu list-traversal primitives (keeps list
next intact) and guarantees concurrent nvmet_find_naespace forward
progress.

By changing that, we cannot rely on ns->dev_link for knowing if the
namspace is enabled, so add enabled indicator entry to nvmet_ns for
that.
Signed-off-by: default avatarSagi Grimberg <sagi@grimberg.me>
Signed-off-by: default avatarSolganik Alexander <sashas@lightbitslabs.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 6290a3bc
...@@ -271,7 +271,7 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item, ...@@ -271,7 +271,7 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
mutex_lock(&subsys->lock); mutex_lock(&subsys->lock);
ret = -EBUSY; ret = -EBUSY;
if (nvmet_ns_enabled(ns)) if (ns->enabled)
goto out_unlock; goto out_unlock;
kfree(ns->device_path); kfree(ns->device_path);
...@@ -307,7 +307,7 @@ static ssize_t nvmet_ns_device_nguid_store(struct config_item *item, ...@@ -307,7 +307,7 @@ static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
int ret = 0; int ret = 0;
mutex_lock(&subsys->lock); mutex_lock(&subsys->lock);
if (nvmet_ns_enabled(ns)) { if (ns->enabled) {
ret = -EBUSY; ret = -EBUSY;
goto out_unlock; goto out_unlock;
} }
...@@ -339,7 +339,7 @@ CONFIGFS_ATTR(nvmet_ns_, device_nguid); ...@@ -339,7 +339,7 @@ CONFIGFS_ATTR(nvmet_ns_, device_nguid);
static ssize_t nvmet_ns_enable_show(struct config_item *item, char *page) static ssize_t nvmet_ns_enable_show(struct config_item *item, char *page)
{ {
return sprintf(page, "%d\n", nvmet_ns_enabled(to_nvmet_ns(item))); return sprintf(page, "%d\n", to_nvmet_ns(item)->enabled);
} }
static ssize_t nvmet_ns_enable_store(struct config_item *item, static ssize_t nvmet_ns_enable_store(struct config_item *item,
......
...@@ -264,7 +264,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns) ...@@ -264,7 +264,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
int ret = 0; int ret = 0;
mutex_lock(&subsys->lock); mutex_lock(&subsys->lock);
if (!list_empty(&ns->dev_link)) if (ns->enabled)
goto out_unlock; goto out_unlock;
ns->bdev = blkdev_get_by_path(ns->device_path, FMODE_READ | FMODE_WRITE, ns->bdev = blkdev_get_by_path(ns->device_path, FMODE_READ | FMODE_WRITE,
...@@ -309,6 +309,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns) ...@@ -309,6 +309,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0); nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0);
ns->enabled = true;
ret = 0; ret = 0;
out_unlock: out_unlock:
mutex_unlock(&subsys->lock); mutex_unlock(&subsys->lock);
...@@ -325,11 +326,11 @@ void nvmet_ns_disable(struct nvmet_ns *ns) ...@@ -325,11 +326,11 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
struct nvmet_ctrl *ctrl; struct nvmet_ctrl *ctrl;
mutex_lock(&subsys->lock); mutex_lock(&subsys->lock);
if (list_empty(&ns->dev_link)) { if (!ns->enabled)
mutex_unlock(&subsys->lock); goto out_unlock;
return;
} ns->enabled = false;
list_del_init(&ns->dev_link); list_del_rcu(&ns->dev_link);
mutex_unlock(&subsys->lock); mutex_unlock(&subsys->lock);
/* /*
...@@ -351,6 +352,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns) ...@@ -351,6 +352,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
if (ns->bdev) if (ns->bdev)
blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ); blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
out_unlock:
mutex_unlock(&subsys->lock); mutex_unlock(&subsys->lock);
} }
......
...@@ -47,6 +47,7 @@ struct nvmet_ns { ...@@ -47,6 +47,7 @@ struct nvmet_ns {
loff_t size; loff_t size;
u8 nguid[16]; u8 nguid[16];
bool enabled;
struct nvmet_subsys *subsys; struct nvmet_subsys *subsys;
const char *device_path; const char *device_path;
...@@ -61,11 +62,6 @@ static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item) ...@@ -61,11 +62,6 @@ static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
return container_of(to_config_group(item), struct nvmet_ns, group); return container_of(to_config_group(item), struct nvmet_ns, group);
} }
static inline bool nvmet_ns_enabled(struct nvmet_ns *ns)
{
return !list_empty_careful(&ns->dev_link);
}
struct nvmet_cq { struct nvmet_cq {
u16 qid; u16 qid;
u16 size; u16 size;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment