nvmet: Fix possible infinite loop triggered on hot namespace removal

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: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Solganik Alexander <sashas@lightbitslabs.com>
Cc: <stable@vger.kernel.org> # v4.8+

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index af5e2dc4a3d503bb8bca93951bcfa3be58685ca5..011f88e5663e72591ad60f5ed21a97682ad2b7e2 100644 (file)
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -271,7 +271,7 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
 
        mutex_lock(&subsys->lock);
        ret = -EBUSY;
-       if (nvmet_ns_enabled(ns))
+       if (ns->enabled)
                goto out_unlock;
 
        kfree(ns->device_path);
@@ -307,7 +307,7 @@ static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
        int ret = 0;
 
        mutex_lock(&subsys->lock);
-       if (nvmet_ns_enabled(ns)) {
+       if (ns->enabled) {
                ret = -EBUSY;
                goto out_unlock;
        }
@@ -339,7 +339,7 @@ CONFIGFS_ATTR(nvmet_ns_, device_nguid);
 
 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,

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index c232552be2d8073b8eafdb67778ea9725bb70225..8b627e2a55c6d65a841d4b3e84c9baaa25a097e8 100644 (file)
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -264,7 +264,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
        int ret = 0;
 
        mutex_lock(&subsys->lock);
-       if (!list_empty(&ns->dev_link))
+       if (ns->enabled)
                goto out_unlock;
 
        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)
        list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
                nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0);
 
+       ns->enabled = true;
        ret = 0;
 out_unlock:
        mutex_unlock(&subsys->lock);
@@ -325,11 +326,11 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
        struct nvmet_ctrl *ctrl;
 
        mutex_lock(&subsys->lock);
-       if (list_empty(&ns->dev_link)) {
-               mutex_unlock(&subsys->lock);
-               return;
-       }
-       list_del_init(&ns->dev_link);
+       if (!ns->enabled)
+               goto out_unlock;
+
+       ns->enabled = false;
+       list_del_rcu(&ns->dev_link);
        mutex_unlock(&subsys->lock);
 
        /*
@@ -351,6 +352,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
 
        if (ns->bdev)
                blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
+out_unlock:
        mutex_unlock(&subsys->lock);
 }
 

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f9c76441e8c941604f8bf83a6f1cb1e8a8f53903..23d5eb1c944f64c485fef8551a41a72151492f2c 100644 (file)
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -47,6 +47,7 @@ struct nvmet_ns {
        loff_t                  size;
        u8                      nguid[16];
 
+       bool                    enabled;
        struct nvmet_subsys     *subsys;
        const char              *device_path;
 
@@ -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);
 }
 
-static inline bool nvmet_ns_enabled(struct nvmet_ns *ns)
-{
-       return !list_empty_careful(&ns->dev_link);
-}
-
 struct nvmet_cq {
        u16                     qid;
        u16                     size;