This is a note to let you know that I've just added the patch titled
nvme-fabrics: protect against module unload during create_ctrl
to the 4.15-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: nvme-fabrics-protect-against-module-unload-during-create_ctrl.patch and it can be found in the queue-4.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Mon Apr 9 10:16:32 CEST 2018
From: Roy Shterman roys@lightbitslabs.com Date: Mon, 25 Dec 2017 14:18:30 +0200 Subject: nvme-fabrics: protect against module unload during create_ctrl
From: Roy Shterman roys@lightbitslabs.com
[ Upstream commit 0de5cd367c6aa2a31a1c931628f778f79f8ef22e ]
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 Signed-off-by: Sasha Levin alexander.levin@microsoft.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/nvme/host/fabrics.c | 17 +++++++++++++---- drivers/nvme/host/fabrics.h | 2 ++ drivers/nvme/host/fc.c | 1 + drivers/nvme/host/rdma.c | 1 + drivers/nvme/target/loop.c | 1 + 5 files changed, 18 insertions(+), 4 deletions(-)
--- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,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); @@ -869,32 +869,41 @@ nvmf_create_ctrl(struct device *dev, con 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: --- 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; --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -3380,6 +3380,7 @@ nvme_fc_create_ctrl(struct device *dev,
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, --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -2018,6 +2018,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, --- a/drivers/nvme/target/loop.c +++ b/drivers/nvme/target/loop.c @@ -686,6 +686,7 @@ static struct nvmet_fabrics_ops nvme_loo
static struct nvmf_transport_ops nvme_loop_transport = { .name = "loop", + .module = THIS_MODULE, .create_ctrl = nvme_loop_create_ctrl, };
Patches currently in stable-queue which might be from roys@lightbitslabs.com are
queue-4.15/nvme-fabrics-protect-against-module-unload-during-create_ctrl.patch
--- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,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);
Hi Greg,
I think that this part broke builtin compilation of nvme over fabrics code.
This was later fixed by Christoph in: -- commit 5a1e59533380a3fd04593e4ab2d4633ebf7745c1 Author: Christoph Hellwig hch@lst.de Date: Thu Feb 22 07:24:08 2018 -0800
nvme-fabrics: don't check for non-NULL module in nvmf_register_transport
THIS_MODULE evaluates to NULL when used from code built into the kernel, thus breaking built-in transport modules. Remove the bogus check.
Fixes: 0de5cd36 ("nvme-fabrics: protect against module unload during create_ctrl") Signed-off-by: Christoph Hellwig hch@lst.de Reviewed-by: Sagi Grimberg sagi@grimberg.me Reviewed-by: Johannes Thumshirn jthumshirn@suse.de Signed-off-by: Keith Busch keith.busch@intel.com
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 5dd4ceefed8f..a1c58e35075e 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect); */ int nvmf_register_transport(struct nvmf_transport_ops *ops) { - if (!ops->create_ctrl || !ops->module) + if (!ops->create_ctrl) return -EINVAL;
down_write(&nvmf_transports_rwsem); --
So I'd suggest taking that as well.
On Mon, Apr 09, 2018 at 01:37:11PM +0300, Sagi Grimberg wrote:
--- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,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);
Hi Greg,
I think that this part broke builtin compilation of nvme over fabrics code.
This was later fixed by Christoph in:
commit 5a1e59533380a3fd04593e4ab2d4633ebf7745c1 Author: Christoph Hellwig hch@lst.de Date: Thu Feb 22 07:24:08 2018 -0800
nvme-fabrics: don't check for non-NULL module in nvmf_register_transport THIS_MODULE evaluates to NULL when used from code built into the kernel, thus breaking built-in transport modules. Remove the bogus check. Fixes: 0de5cd36 ("nvme-fabrics: protect against module unload during
create_ctrl") Signed-off-by: Christoph Hellwig hch@lst.de Reviewed-by: Sagi Grimberg sagi@grimberg.me Reviewed-by: Johannes Thumshirn jthumshirn@suse.de Signed-off-by: Keith Busch keith.busch@intel.com
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c index 5dd4ceefed8f..a1c58e35075e 100644 --- a/drivers/nvme/host/fabrics.c +++ b/drivers/nvme/host/fabrics.c @@ -493,7 +493,7 @@ EXPORT_SYMBOL_GPL(nvmf_should_reconnect); */ int nvmf_register_transport(struct nvmf_transport_ops *ops) {
if (!ops->create_ctrl || !ops->module)
if (!ops->create_ctrl) return -EINVAL; down_write(&nvmf_transports_rwsem);
--
So I'd suggest taking that as well.
Many thanks for letting me know, I've now queued up that patch as well.
greg k-h
linux-stable-mirror@lists.linaro.org