nvme-fabrics: protect against module unload during create_ctrl

NVMe transport driver module unload may (and usually does) trigger
iteration over the active controllers and delete them all (sometimes
under a mutex).  However, a controller can be created concurrently with
module unload which can lead to leakage of resources (most important char
device node leakage) in case the controller creation occured after the
unload delete and drain sequence.  To protect against this, we take a
module reference to guarantee that the nvme transport driver is not
unloaded while creating a controller.

Signed-off-by: Roy Shterman <roys@lightbitslabs.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 76b4fe6816a03533dbb6bbfbf30016110a26c648..2f68befd31bf5fcb66559d3fb563730c22ac9d30 100644 (file)
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -492,7 +492,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect);
  */
 int nvmf_register_transport(struct nvmf_transport_ops *ops)
 {
-       if (!ops->create_ctrl)
+       if (!ops->create_ctrl || !ops->module)
                return -EINVAL;
 
        down_write(&nvmf_transports_rwsem);
@@ -868,32 +868,41 @@ nvmf_create_ctrl(struct device *dev, const char *buf, size_t count)
                goto out_unlock;
        }
 
+       if (!try_module_get(ops->module)) {
+               ret = -EBUSY;
+               goto out_unlock;
+       }
+
        ret = nvmf_check_required_opts(opts, ops->required_opts);
        if (ret)
-               goto out_unlock;
+               goto out_module_put;
        ret = nvmf_check_allowed_opts(opts, NVMF_ALLOWED_OPTS |
                                ops->allowed_opts | ops->required_opts);
        if (ret)
-               goto out_unlock;
+               goto out_module_put;
 
        ctrl = ops->create_ctrl(dev, opts);
        if (IS_ERR(ctrl)) {
                ret = PTR_ERR(ctrl);
-               goto out_unlock;
+               goto out_module_put;
        }
 
        if (strcmp(ctrl->subsys->subnqn, opts->subsysnqn)) {
                dev_warn(ctrl->device,
                        "controller returned incorrect NQN: \"%s\".\n",
                        ctrl->subsys->subnqn);
+               module_put(ops->module);
                up_read(&nvmf_transports_rwsem);
                nvme_delete_ctrl_sync(ctrl);
                return ERR_PTR(-EINVAL);
        }
 
+       module_put(ops->module);
        up_read(&nvmf_transports_rwsem);
        return ctrl;
 
+out_module_put:
+       module_put(ops->module);
 out_unlock:
        up_read(&nvmf_transports_rwsem);
 out_free_opts:

diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index 9ba614953607eba072000fb10c2854ea1ee677d6..25b19f722f5b20508ed35d408305dde734498432 100644 (file)
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -108,6 +108,7 @@ struct nvmf_ctrl_options {
  *                            fabric implementation of NVMe fabrics.
  * @entry:             Used by the fabrics library to add the new
  *                     registration entry to its linked-list internal tree.
+ * @module:             Transport module reference
  * @name:              Name of the NVMe fabric driver implementation.
  * @required_opts:     sysfs command-line options that must be specified
  *                     when adding a new NVMe controller.
@@ -126,6 +127,7 @@ struct nvmf_ctrl_options {
  */
 struct nvmf_transport_ops {
        struct list_head        entry;
+       struct module           *module;
        const char              *name;
        int                     required_opts;
        int                     allowed_opts;

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 0a8af4daef8903f8ba983d345f1044498c57a975..2a7a9a75105d7df0072f2a87b6ef674d607523c8 100644 (file)
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3381,6 +3381,7 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts)
 
 static struct nvmf_transport_ops nvme_fc_transport = {
        .name           = "fc",
+       .module         = THIS_MODULE,
        .required_opts  = NVMF_OPT_TRADDR | NVMF_OPT_HOST_TRADDR,
        .allowed_opts   = NVMF_OPT_RECONNECT_DELAY | NVMF_OPT_CTRL_LOSS_TMO,
        .create_ctrl    = nvme_fc_create_ctrl,

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 37af56596be6ce8a0339ce2a3151f8dd98f8f854..75d6956eb380c89d8566ac91f6a964919361885b 100644 (file)
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -2006,6 +2006,7 @@ out_free_ctrl:
 
 static struct nvmf_transport_ops nvme_rdma_transport = {
        .name           = "rdma",
+       .module         = THIS_MODULE,
        .required_opts  = NVMF_OPT_TRADDR,
        .allowed_opts   = NVMF_OPT_TRSVCID | NVMF_OPT_RECONNECT_DELAY |
                          NVMF_OPT_HOST_TRADDR | NVMF_OPT_CTRL_LOSS_TMO,

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 1e21b286f299834298fb9d23f06f9f20e0797157..fdfcc961029f9844d84114b1c096426cb17e10ae 100644 (file)
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -686,6 +686,7 @@ static struct nvmet_fabrics_ops nvme_loop_ops = {
 
 static struct nvmf_transport_ops nvme_loop_transport = {
        .name           = "loop",
+       .module         = THIS_MODULE,
        .create_ctrl    = nvme_loop_create_ctrl,
 };