Commit 0de5cd36 authored by Roy Shterman's avatar Roy Shterman Committed by Christoph Hellwig

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: default avatarRoy Shterman <roys@lightbitslabs.com>
Signed-off-by: default avatarSagi Grimberg <sagi@grimberg.me>
Reviewed-by: default avatarMax Gurtovoy <maxg@mellanox.com>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent 9ce1f2e1
...@@ -492,7 +492,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect); ...@@ -492,7 +492,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect);
*/ */
int nvmf_register_transport(struct nvmf_transport_ops *ops) int nvmf_register_transport(struct nvmf_transport_ops *ops)
{ {
if (!ops->create_ctrl) if (!ops->create_ctrl || !ops->module)
return -EINVAL; return -EINVAL;
down_write(&nvmf_transports_rwsem); down_write(&nvmf_transports_rwsem);
...@@ -868,32 +868,41 @@ nvmf_create_ctrl(struct device *dev, const char *buf, size_t count) ...@@ -868,32 +868,41 @@ nvmf_create_ctrl(struct device *dev, const char *buf, size_t count)
goto out_unlock; goto out_unlock;
} }
if (!try_module_get(ops->module)) {
ret = -EBUSY;
goto out_unlock;
}
ret = nvmf_check_required_opts(opts, ops->required_opts); ret = nvmf_check_required_opts(opts, ops->required_opts);
if (ret) if (ret)
goto out_unlock; goto out_module_put;
ret = nvmf_check_allowed_opts(opts, NVMF_ALLOWED_OPTS | ret = nvmf_check_allowed_opts(opts, NVMF_ALLOWED_OPTS |
ops->allowed_opts | ops->required_opts); ops->allowed_opts | ops->required_opts);
if (ret) if (ret)
goto out_unlock; goto out_module_put;
ctrl = ops->create_ctrl(dev, opts); ctrl = ops->create_ctrl(dev, opts);
if (IS_ERR(ctrl)) { if (IS_ERR(ctrl)) {
ret = PTR_ERR(ctrl); ret = PTR_ERR(ctrl);
goto out_unlock; goto out_module_put;
} }
if (strcmp(ctrl->subsys->subnqn, opts->subsysnqn)) { if (strcmp(ctrl->subsys->subnqn, opts->subsysnqn)) {
dev_warn(ctrl->device, dev_warn(ctrl->device,
"controller returned incorrect NQN: \"%s\".\n", "controller returned incorrect NQN: \"%s\".\n",
ctrl->subsys->subnqn); ctrl->subsys->subnqn);
module_put(ops->module);
up_read(&nvmf_transports_rwsem); up_read(&nvmf_transports_rwsem);
nvme_delete_ctrl_sync(ctrl); nvme_delete_ctrl_sync(ctrl);
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
} }
module_put(ops->module);
up_read(&nvmf_transports_rwsem); up_read(&nvmf_transports_rwsem);
return ctrl; return ctrl;
out_module_put:
module_put(ops->module);
out_unlock: out_unlock:
up_read(&nvmf_transports_rwsem); up_read(&nvmf_transports_rwsem);
out_free_opts: out_free_opts:
......
...@@ -108,6 +108,7 @@ struct nvmf_ctrl_options { ...@@ -108,6 +108,7 @@ struct nvmf_ctrl_options {
* fabric implementation of NVMe fabrics. * fabric implementation of NVMe fabrics.
* @entry: Used by the fabrics library to add the new * @entry: Used by the fabrics library to add the new
* registration entry to its linked-list internal tree. * registration entry to its linked-list internal tree.
* @module: Transport module reference
* @name: Name of the NVMe fabric driver implementation. * @name: Name of the NVMe fabric driver implementation.
* @required_opts: sysfs command-line options that must be specified * @required_opts: sysfs command-line options that must be specified
* when adding a new NVMe controller. * when adding a new NVMe controller.
...@@ -126,6 +127,7 @@ struct nvmf_ctrl_options { ...@@ -126,6 +127,7 @@ struct nvmf_ctrl_options {
*/ */
struct nvmf_transport_ops { struct nvmf_transport_ops {
struct list_head entry; struct list_head entry;
struct module *module;
const char *name; const char *name;
int required_opts; int required_opts;
int allowed_opts; int allowed_opts;
......
...@@ -3381,6 +3381,7 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts) ...@@ -3381,6 +3381,7 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts)
static struct nvmf_transport_ops nvme_fc_transport = { static struct nvmf_transport_ops nvme_fc_transport = {
.name = "fc", .name = "fc",
.module = THIS_MODULE,
.required_opts = NVMF_OPT_TRADDR | NVMF_OPT_HOST_TRADDR, .required_opts = NVMF_OPT_TRADDR | NVMF_OPT_HOST_TRADDR,
.allowed_opts = NVMF_OPT_RECONNECT_DELAY | NVMF_OPT_CTRL_LOSS_TMO, .allowed_opts = NVMF_OPT_RECONNECT_DELAY | NVMF_OPT_CTRL_LOSS_TMO,
.create_ctrl = nvme_fc_create_ctrl, .create_ctrl = nvme_fc_create_ctrl,
......
...@@ -2006,6 +2006,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev, ...@@ -2006,6 +2006,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
static struct nvmf_transport_ops nvme_rdma_transport = { static struct nvmf_transport_ops nvme_rdma_transport = {
.name = "rdma", .name = "rdma",
.module = THIS_MODULE,
.required_opts = NVMF_OPT_TRADDR, .required_opts = NVMF_OPT_TRADDR,
.allowed_opts = NVMF_OPT_TRSVCID | NVMF_OPT_RECONNECT_DELAY | .allowed_opts = NVMF_OPT_TRSVCID | NVMF_OPT_RECONNECT_DELAY |
NVMF_OPT_HOST_TRADDR | NVMF_OPT_CTRL_LOSS_TMO, NVMF_OPT_HOST_TRADDR | NVMF_OPT_CTRL_LOSS_TMO,
......
...@@ -686,6 +686,7 @@ static struct nvmet_fabrics_ops nvme_loop_ops = { ...@@ -686,6 +686,7 @@ static struct nvmet_fabrics_ops nvme_loop_ops = {
static struct nvmf_transport_ops nvme_loop_transport = { static struct nvmf_transport_ops nvme_loop_transport = {
.name = "loop", .name = "loop",
.module = THIS_MODULE,
.create_ctrl = nvme_loop_create_ctrl, .create_ctrl = nvme_loop_create_ctrl,
}; };
......
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