[PATCH] nvme-core: Fixing ctrl de-referencing in nvme_init_ctrl
Chaitanya Kulkarni
chaitanyak at nvidia.com
Thu Apr 13 02:20:10 PDT 2023
On 4/2/23 17:41, Irvin Cote wrote:
> Correcting the de-reference count for
> the controller in the core layer to avoid
> a memory leak. Also cleaning a bit the
> teardown logic of nvme_init_ctrl.
>
> Signed-off-by: Irvin Cote <irvincoteg at gmail.com>
> ---
> drivers/nvme/host/core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 8698410aeb84..3d7aca7d0f2a 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -5137,14 +5137,12 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> 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;
> + goto out_free_page;
> ctrl->instance = ret;
>
> device_initialize(&ctrl->ctrl_device);
> @@ -5191,10 +5189,10 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> nvme_put_ctrl(ctrl);
> kfree_const(ctrl->device->kobj.name);
> out_release_instance:
> + nvme_put_ctrl(ctrl);
> ida_free(&nvme_instance_ida, ctrl->instance);
> -out:
> - if (ctrl->discard_page)
> - __free_page(ctrl->discard_page);
> +out_free_page:
> + __free_page(ctrl->discard_page);
> return ret;
> }
> EXPORT_SYMBOL_GPL(nvme_init_ctrl);
blktests nvme/044 is failing, potential problem commit by just eye
inspection,
3250e769508b59b1ca54d9892bd8683afc684c86
("nvme: fix and error path leak in nvme_init_ctr") :-
<4>[ 549.551854] ------------[ cut here ]------------
<4>[ 549.551855] ida_free called for id=1 which is not allocated.
<4>[ 549.551870] WARNING: CPU: 18 PID: 3593 at lib/idr.c:525
ida_free+0xec/0x130
<4>[ 549.551917] CPU: 18 PID: 3593 Comm: nvme Tainted: G W
O N 6.3.0-rc2nvme+ #1
<4>[ 549.551919] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
<4>[ 549.551920] RIP: 0010:ida_free+0xec/0x130
<4>[ 549.551926] RSP: 0018:ffffc900014abce0 EFLAGS: 00010286
<4>[ 549.551928] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
0000000000000000
<4>[ 549.551932] RDX: 0000000000000002 RSI: ffffffff8272f833 RDI:
00000000ffffffff
<4>[ 549.551937] RBP: 0000000000000001 R08: ffff88983ff3cba8 R09:
00000000ffffbfff
<4>[ 549.551938] R10: ffff8897df0a0000 R11: ffff88983fedcbc0 R12:
0000000000000001
<4>[ 549.551939] R13: 0000000000000202 R14: ffff8881044000b0 R15:
ffff88810477c000
<4>[ 549.551942] FS: 00007f1cb198ab80(0000) GS:ffff8897df680000(0000)
knlGS:0000000000000000
<4>[ 549.551944] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 549.551945] CR2: 0000000000b7b0c8 CR3: 0000000162a1c000 CR4:
0000000000350ee0
<4>[ 549.551948] DR0: ffffffff84379428 DR1: ffffffff84379429 DR2:
ffffffff8437942a
<4>[ 549.551950] DR3: ffffffff8437942b DR6: 00000000ffff0ff0 DR7:
0000000000000600
<4>[ 549.551951] Call Trace:
<4>[ 549.551952] <TASK>
<4>[ 549.551955] nvme_init_ctrl+0x3a3/0x530 [nvme_core]
<4>[ 549.551970] nvme_loop_create_ctrl+0xa3/0x3f0 [nvme_loop]
<4>[ 549.551976] nvmf_dev_write+0x5db/0xe80 [nvme_fabrics]
<4>[ 549.551983] ? inode_security+0x22/0x60
<4>[ 549.551985] ? selinux_file_permission+0x108/0x150
<4>[ 549.551987] vfs_write+0xc5/0x3c0
<4>[ 549.551990] ? _raw_spin_unlock+0x15/0x30
<4>[ 549.551992] ? preempt_count_add+0x4d/0xa0
<4>[ 549.551994] ? fd_install+0x5c/0xe0
<4>[ 549.551997] ksys_write+0x5f/0xe0
<4>[ 549.551999] do_syscall_64+0x3b/0x90
<4>[ 549.552000] entry_SYSCALL_64_after_hwframe+0x72/0xdc
<4>[ 549.552002] RIP: 0033:0x7f1cb1aa07a7
<4>[ 549.552006] RSP: 002b:00007ffdc6b16898 EFLAGS: 00000246 ORIG_RAX:
0000000000000001
<4>[ 549.552007] RAX: ffffffffffffffda RBX: 0000000000b7a090 RCX:
00007f1cb1aa07a7
<4>[ 549.552008] RDX: 0000000000000139 RSI: 0000000000b7a090 RDI:
0000000000000004
<4>[ 549.552008] RBP: 0000000000000004 R08: 0000000000000139 R09:
0000000000b7a090
<4>[ 549.552009] R10: 00007f1cb19c4118 R11: 0000000000000246 R12:
0000000000b787e0
<4>[ 549.552010] R13: 0000000000000139 R14: 00007f1cb1bcd11d R15:
00007f1cb1bcd02b
<4>[ 549.552011] </TASK>
<4>[ 549.552014] ---[ end trace 0000000000000000 ]---
<6>[ 549.552021] nvme_init_ctrl 5202
<6>[ 549.552022] nvme_init_ctrl 5204
<4>[ 549.552022] page:00000000a1e1cf34 refcount:0 mapcount:0
mapping:0000000000000000 index:0x0 pfn:0x15975a
<4>[ 549.552025] flags:
0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
<4>[ 549.552027] raw: 0017ffffc0000000 ffffea0004889dc8
ffff8897df6b3158 0000000000000000
<4>[ 549.552028] raw: 0000000000000000 0000000000000000
00000000ffffffff 0000000000000000
<4>[ 549.552029] page dumped because:
VM_BUG_ON_PAGE(page_ref_count(page) == 0)
<4>[ 549.552033] ------------[ cut here ]------------
<2>[ 549.552034] kernel BUG at include/linux/mm.h:875!
[18]kdb>
* with the following fix its working :-
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 518c759346f0..b59570e50657 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -5157,7 +5157,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct
device *dev,
dev_set_drvdata(ctrl->device, ctrl);
ret = dev_set_name(ctrl->device, "nvme%d", ctrl->instance);
if (ret)
- goto out_put_ctrl;
+ goto out_release_instance;
nvme_get_ctrl(ctrl);
cdev_init(&ctrl->cdev, &nvme_dev_fops);
@@ -5186,8 +5186,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct
device *dev,
out_free_name:
nvme_put_ctrl(ctrl);
kfree_const(ctrl->device->kobj.name);
-out_put_ctrl:
- nvme_put_ctrl(ctrl);
+out_release_instance:
ida_free(&nvme_instance_ida, ctrl->instance);
out_free_page:
__free_page(ctrl->discard_page);
Perhaps we can drop original change for now and reconsider later ...
-ck
More information about the Linux-nvme
mailing list