nvme: Fix ctrl use-after-free during sysfs deletion

In case nvme_sysfs_delete() is called by the user before taking the ctrl
reference count, the ctrl may be freed during the creation and cause the
bug. Take the reference as soon as the controller is externally visible,
which is done by cdev_device_add() in nvme_init_ctrl(). Also take the
reference count at the core layer instead of taking it on each transport
separately.

Signed-off-by: Israel Rukshin <israelr@mellanox.com>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8a7761c3086e9dfb04c9ec74e3a4deed1cc45ecf..51f80be0fe90c2e5578c45b1fa121e00e80944a9 100644 (file)
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4130,6 +4130,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
        if (ret)
                goto out_release_instance;
 
+       nvme_get_ctrl(ctrl);
        cdev_init(&ctrl->cdev, &nvme_dev_fops);
        ctrl->cdev.owner = ops->module;
        ret = cdev_device_add(&ctrl->cdev, ctrl->device);
@@ -4148,6 +4149,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 
        return 0;
 out_free_name:
+       nvme_put_ctrl(ctrl);
        kfree_const(ctrl->device->kobj.name);
 out_release_instance:
        ida_simple_remove(&nvme_instance_ida, ctrl->instance);

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 5a70ac395d53a0f724f3f29431c4c32afa235619..59d2e2bec17999cb9457ccdce4d1e13755b8b53a 100644 (file)
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3181,10 +3181,7 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
                goto fail_ctrl;
        }
 
-       nvme_get_ctrl(&ctrl->ctrl);
-
        if (!queue_delayed_work(nvme_wq, &ctrl->connect_work, 0)) {
-               nvme_put_ctrl(&ctrl->ctrl);
                dev_err(ctrl->ctrl.device,
                        "NVME-FC{%d}: failed to schedule initial connect\n",
                        ctrl->cnum);
@@ -3209,6 +3206,7 @@ fail_ctrl:
 
        /* initiate nvme ctrl ref counting teardown */
        nvme_uninit_ctrl(&ctrl->ctrl);
+       nvme_put_ctrl(&ctrl->ctrl);
 
        /* Remove core ctrl ref. */
        nvme_put_ctrl(&ctrl->ctrl);

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ff0bd2d84f3e131a366c73275665771166c1fce8..4e062c3a84bcc8e33b79d264e86b5520b17cffec 100644 (file)
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2802,7 +2802,6 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
        dev_info(dev->ctrl.device, "pci function %s\n", dev_name(&pdev->dev));
 
        nvme_reset_ctrl(&dev->ctrl);
-       nvme_get_ctrl(&dev->ctrl);
        async_schedule(nvme_async_probe, dev);
 
        return 0;

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 3e85c5cacefd25f742def056fc6d485443a5345f..ca782deea72dfd9acecf9561491bd054676ac457 100644 (file)
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -2043,8 +2043,6 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
        dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n",
                ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
 
-       nvme_get_ctrl(&ctrl->ctrl);
-
        mutex_lock(&nvme_rdma_ctrl_mutex);
        list_add_tail(&ctrl->list, &nvme_rdma_ctrl_list);
        mutex_unlock(&nvme_rdma_ctrl_mutex);
@@ -2054,6 +2052,7 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 out_uninit_ctrl:
        nvme_uninit_ctrl(&ctrl->ctrl);
        nvme_put_ctrl(&ctrl->ctrl);
+       nvme_put_ctrl(&ctrl->ctrl);
        if (ret > 0)
                ret = -EIO;
        return ERR_PTR(ret);

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 4b20301e517ca46fe68406bccf9b01eafcbb3264..dd569b122a0d699c789e15e4f906d393e6a09aa0 100644 (file)
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2428,8 +2428,6 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
        dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISp\n",
                ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
 
-       nvme_get_ctrl(&ctrl->ctrl);
-
        mutex_lock(&nvme_tcp_ctrl_mutex);
        list_add_tail(&ctrl->list, &nvme_tcp_ctrl_list);
        mutex_unlock(&nvme_tcp_ctrl_mutex);
@@ -2439,6 +2437,7 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
 out_uninit_ctrl:
        nvme_uninit_ctrl(&ctrl->ctrl);
        nvme_put_ctrl(&ctrl->ctrl);
+       nvme_put_ctrl(&ctrl->ctrl);
        if (ret > 0)
                ret = -EIO;
        return ERR_PTR(ret);

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 4df4ebde208a0465dac1e975304fe4a6fd358c2f..a425e2858829b52898deba3b8a47e1773fe61606 100644 (file)
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -618,8 +618,6 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
        dev_info(ctrl->ctrl.device,
                 "new ctrl: \"%s\"\n", ctrl->ctrl.opts->subsysnqn);
 
-       nvme_get_ctrl(&ctrl->ctrl);
-
        changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
        WARN_ON_ONCE(!changed);
 
@@ -637,6 +635,7 @@ out_free_queues:
        kfree(ctrl->queues);
 out_uninit_ctrl:
        nvme_uninit_ctrl(&ctrl->ctrl);
+       nvme_put_ctrl(&ctrl->ctrl);
 out_put_ctrl:
        nvme_put_ctrl(&ctrl->ctrl);
        if (ret > 0)