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 8a7761c..51f80be 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 5a70ac3..59d2e2b 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 ff0bd2d..4e062c3 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 3e85c5c..ca782de 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 4b20301..dd569b1 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 4df4ebd..a425e28 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)