[PATCH] nvme: Fix memory leak in nvme_init_ctrl error path

Sagi Grimberg sagi at grimberg.me
Wed May 3 08:09:25 PDT 2023


nvme_init_ctrl may fail before creating the misc device or after creating
the misc device.

If we fail before creating the misc device, we just need to deallocate what
was allocated before and return (as usually done).

If we fail after we create the misc device, we must put
the final reference on the device in order to make sure that internal
device resources are cleaned up.

The device release also triggers nvme_free_ctrl method so we need to make
sure to identify that we failed during the initialization itself and skip
cleaning because nvme_init_ctrl error path cleaned up on its own (we do
that by marking the ctrl->instance to UNINITIALIZED).

We also drop the explicit dev_name deallocation because it is freed in
the device release sequence.

This addresses a memory leak triggered by blktests nvme/044 which happens
to fail exactly after misc device initialization:
--
unreferenced object 0xffff95678a54cd00 (size 256):
  comm "nvme", pid 1941, jiffies 4294761594 (age 10.010s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 08 cd 54 8a 67 95 ff ff  ..........T.g...
    08 cd 54 8a 67 95 ff ff e0 b5 7b 8b ff ff ff ff  ..T.g.....{.....
  backtrace:
    [<ffffffff8b349205>] kmalloc_trace+0x25/0x90
    [<ffffffff8b7c0463>] device_add+0x303/0x690
    [<ffffffff8b4103c4>] cdev_device_add+0x44/0x90
    [<ffffffffc0de1c42>] 0xffffffffc0de1c42
    [<ffffffffc0d788b3>] 0xffffffffc0d788b3
    [<ffffffffc0d8c66d>] 0xffffffffc0d8c66d
    [<ffffffffc0d8c831>] 0xffffffffc0d8c831
    [<ffffffff8b40a8b2>] vfs_write+0xc2/0x3c0
    [<ffffffff8b40aeff>] ksys_write+0x5f/0xe0
    [<ffffffff8bc0eb58>] do_syscall_64+0x38/0x90
    [<ffffffff8be000aa>] entry_SYSCALL_64_after_hwframe+0x72/0xdc
--

Reported-by: Irvin Cote <irvincoteg at gmail.com>
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 28 ++++++++++++++++------------
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ea508e9a1de7..b374e6007553 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -5083,6 +5083,9 @@ static void nvme_free_ctrl(struct device *dev)
 		container_of(dev, struct nvme_ctrl, ctrl_device);
 	struct nvme_subsystem *subsys = ctrl->subsys;
 
+	if (ctrl->instance == NVME_CTRL_INSTANCE_UNINITIALIZED)
+		return;
+
 	if (!subsys || ctrl->instance != subsys->instance)
 		ida_free(&nvme_instance_ida, ctrl->instance);
 
@@ -5137,18 +5140,19 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	INIT_DELAYED_WORK(&ctrl->failfast_work, nvme_failfast_work);
 	memset(&ctrl->ka_cmd, 0, sizeof(ctrl->ka_cmd));
 	ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;
+	ctrl->instance = NVME_CTRL_INSTANCE_UNINITIALIZED;
 
 	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
 			PAGE_SIZE);
 	ctrl->discard_page = alloc_page(GFP_KERNEL);
-	if (!ctrl->discard_page) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!ctrl->discard_page)
+		return -ENOMEM;
 
 	ret = ida_alloc(&nvme_instance_ida, GFP_KERNEL);
-	if (ret < 0)
-		goto out;
+	if (ret < 0) {
+		__free_page(ctrl->discard_page);
+		return ret;
+	}
 	ctrl->instance = ret;
 
 	device_initialize(&ctrl->ctrl_device);
@@ -5172,7 +5176,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	ctrl->cdev.owner = ops->module;
 	ret = cdev_device_add(&ctrl->cdev, ctrl->device);
 	if (ret)
-		goto out_free_name;
+		goto out_put_ctrl;
 
 	/*
 	 * Initialize latency tolerance controls.  The sysfs files won't
@@ -5193,14 +5197,14 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	nvme_fault_inject_fini(&ctrl->fault_inject);
 	dev_pm_qos_hide_latency_tolerance(ctrl->device);
 	cdev_device_del(&ctrl->cdev, ctrl->device);
-out_free_name:
+out_put_ctrl:
+	/* pairs with nvme_get_ctrl */
 	nvme_put_ctrl(ctrl);
-	kfree_const(ctrl->device->kobj.name);
 out_release_instance:
 	ida_free(&nvme_instance_ida, ctrl->instance);
-out:
-	if (ctrl->discard_page)
-		__free_page(ctrl->discard_page);
+	ctrl->instance = NVME_CTRL_INSTANCE_UNINITIALIZED;
+	/* pairs with device_initialize .release method will cleanup */
+	nvme_put_ctrl(ctrl);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nvme_init_ctrl);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bf46f122e9e1..920403589670 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -255,6 +255,7 @@ struct nvme_ctrl {
 	struct request_queue *connect_q;
 	struct request_queue *fabrics_q;
 	struct device *dev;
+#define NVME_CTRL_INSTANCE_UNINITIALIZED (-1)
 	int instance;
 	int numa_node;
 	struct blk_mq_tag_set *tagset;
-- 
2.34.1




More information about the Linux-nvme mailing list