[PATCH] nvme-core: remove head->effects to fix use-after-free

From: Yuanyuan Zhong
Date: Wed Nov 15 2023 - 13:55:04 EST


The head->effects stores a pointer to the controller's Command Sets
Supported and Effects log. When the namespace head is shared by multiple
controllers, if the particular controller is removed, dereferencing
head->effects causes use-after-free:

[ 227.820066] ==================================================================
[ 227.820069] BUG: KFENCE: use-after-free read in nvme_command_effects+0x1f/0x90 [nvme_core]

[ 227.820091] Use-after-free read at 0x00000000c02dadcf (in kfence-#0):
[ 227.820094] nvme_command_effects+0x1f/0x90 [nvme_core]
[ 227.820107] nvme_passthru_start+0x19/0x80 [nvme_core]
[ 227.820121] nvme_submit_user_cmd+0xc7/0x170 [nvme_core]
[ 227.820136] nvme_user_cmd.constprop.16+0x152/0x1d0 [nvme_core]
[ 227.820149] nvme_ns_head_ioctl+0x92/0x140 [nvme_core]
[ 227.820162] blkdev_ioctl+0x1c5/0x280
[ 227.820169] __x64_sys_ioctl+0x93/0xd0
[ 227.820174] do_syscall_64+0x45/0xf0
[ 227.820177] entry_SYSCALL_64_after_hwframe+0x6e/0x76

[ 227.820182] kfence-#0: 0x000000000fac1d5d-0x00000000a28a73c3, size=4096, cache=kmalloc-4k

[ 227.820185] allocated by task 2559 on cpu 3 at 188.703118s:
[ 227.820233] __kmem_cache_alloc_node+0x3c9/0x410
[ 227.820236] kmalloc_trace+0x2a/0xa0
[ 227.820238] nvme_get_effects_log+0x68/0xd0 [nvme_core]
[ 227.820251] nvme_init_identify+0x5ef/0x640 [nvme_core]
[ 227.820263] nvme_init_ctrl_finish+0x8d/0x250 [nvme_core]
[ 227.820275] nvme_tcp_setup_ctrl+0x1ce/0x710 [nvme_tcp]
[ 227.820281] nvme_tcp_create_ctrl+0x359/0x450 [nvme_tcp]
[ 227.820286] nvmf_dev_write+0x203/0x3b0 [nvme_fabrics]
[ 227.820292] vfs_write+0xd2/0x3d0
[ 227.820294] ksys_write+0x5d/0xd0
[ 227.820297] do_syscall_64+0x45/0xf0
[ 227.820298] entry_SYSCALL_64_after_hwframe+0x6e/0x76

[ 227.820302] freed by task 2521 on cpu 28 at 220.115945s:
[ 227.820329] nvme_free_ctrl+0xa6/0x260 [nvme_core]
[ 227.820342] device_release+0x37/0x90
[ 227.820345] kobject_release+0x57/0x120
[ 227.820347] nvme_sysfs_delete+0x39/0x50 [nvme_core]
[ 227.820360] kernfs_fop_write_iter+0x144/0x1e0
[ 227.820362] vfs_write+0x25f/0x3d0
[ 227.820364] ksys_write+0x5d/0xd0
[ 227.820366] do_syscall_64+0x45/0xf0
[ 227.820368] entry_SYSCALL_64_after_hwframe+0x6e/0x76

Fix this by removing head->effects. Use the Commands Supported and Effects log
in ctrl->cels directly.

Fixes: be93e87e7802 ("nvme: support for multiple Command Sets Supported and Effects log pages")
Signed-off-by: Yuanyuan Zhong <yzhong@xxxxxxxxxxxxxxx>
Reviewed-by: Randy Jennings <randyj@xxxxxxxxxxxxxxx>
Reviewed-by: Hamilton Coutinho <hcoutinho@xxxxxxxxxxxxxxx>
---
drivers/nvme/host/core.c | 17 ++++++++---------
drivers/nvme/host/nvme.h | 1 -
2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 88b54cdcbd68..14fdc2de3a75 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1085,7 +1085,9 @@ u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
u32 effects = 0;

if (ns) {
- effects = le32_to_cpu(ns->head->effects->iocs[opcode]);
+ struct nvme_effects_log *cel = xa_load(&ctrl->cels, ns->head->ids.csi);
+
+ effects = le32_to_cpu(cel->iocs[opcode]);
if (effects & ~(NVME_CMD_EFFECTS_CSUPP | NVME_CMD_EFFECTS_LBCC))
dev_warn_once(ctrl->device,
"IO command:%02x has unusual effects:%08x\n",
@@ -2872,7 +2874,8 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,

xa_store(&ctrl->cels, csi, cel, GFP_KERNEL);
out:
- *log = cel;
+ if (log)
+ *log = cel;
return 0;
}

@@ -3388,13 +3391,6 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
head->shared = info->is_shared;
kref_init(&head->ref);

- if (head->ids.csi) {
- ret = nvme_get_effects_log(ctrl, head->ids.csi, &head->effects);
- if (ret)
- goto out_cleanup_srcu;
- } else
- head->effects = ctrl->effects;
-
ret = nvme_mpath_alloc_disk(ctrl, head);
if (ret)
goto out_cleanup_srcu;
@@ -3779,6 +3775,9 @@ static void nvme_scan_ns(struct nvme_ctrl *ctrl, unsigned nsid)
if (ret || !info.is_ready)
return;

+ if (info.ids.csi && nvme_get_effects_log(ctrl, info.ids.csi, NULL))
+ return;
+
ns = nvme_find_get_ns(ctrl, nsid);
if (ns) {
nvme_validate_ns(ns, &info);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 39a90b7cb125..38298c072ec3 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -445,7 +445,6 @@ struct nvme_ns_head {
struct kref ref;
bool shared;
int instance;
- struct nvme_effects_log *effects;

struct cdev cdev;
struct device cdev_device;
--
2.34.1