[PATCH] nvme: keep a private copy of the effects log in the ns head

[PATCH] nvme: keep a private copy of the effects log in the ns head

[Yogesh Gaur]

nvme_alloc_ns_head() points head->effects at the creating controller's
Commands Supported and Effects log, which lives in ctrl->cels and is
freed by nvme_free_ctrl(). The head is owned by the subsystem, though:
with several controllers on one subsystem it is shared between them
and stays around after the controller that allocated it is gone. When
another controller then (re)scans the namespace, nvme_query_zone_info()
and nvme_command_effects() read the freed log:

  BUG: KASAN: slab-use-after-free in nvme_query_zone_info+0x4fd/0x6f0 drivers/nvme/host/zns.c:47
  Read of size 4 at addr ffff88802ab9e5f4 by task kworker/u8:2/43
  Workqueue: async async_run_entry_fn
   nvme_query_zone_info+0x4fd/0x6f0 drivers/nvme/host/zns.c:47
   nvme_update_ns_info_block+0x1b2e/0x2af0 drivers/nvme/host/core.c:2430
   nvme_update_ns_info+0xc6/0xd90 drivers/nvme/host/core.c:2553
   nvme_alloc_ns+0x1a9f/0x3e50 drivers/nvme/host/core.c:4293
   nvme_scan_ns+0x79d/0x910 drivers/nvme/host/core.c:4483
  Allocated by task 6345:
   nvme_get_effects_log+0x13a/0x280 drivers/nvme/host/core.c:3422
   nvme_alloc_ns_head drivers/nvme/host/core.c:4037 [inline]
   nvme_init_ns_head drivers/nvme/host/core.c:4153 [inline]
  Freed by task 14867:
   nvme_free_cels drivers/nvme/host/core.c:5165 [inline]
   nvme_free_ctrl+0x1e4/0x6b0 drivers/nvme/host/core.c:5183

syzbot hits this with nvme-loop by connecting the same zoned subsystem
several times with duplicate_connect while deleting and rescanning the
controllers. The NVM command set case (head->effects = ctrl->effects)
has the same lifetime problem, as ctrl->effects is also stored in
ctrl->cels.

Give the head its own copy of the log, taken when the head is created
and freed together with it. That keeps today's behaviour of using the
first controller's log for the whole head, but no longer ties the
head's lifetime to that controller.

Fixes: be93e87e7802 ("nvme: support for multiple Command Sets Supported and Effects log pages")
Cc: stable@vger.kernel.org
Reported-by: syzbot+76c0f0ce8f1e846b4f84@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=34a8e8b643a20c2cbde3
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
 drivers/nvme/host/core.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 5d7dfd7a63d4..f0df4c204e52 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -694,6 +694,7 @@ static void nvme_free_ns_head(struct kref *ref)
 	cleanup_srcu_struct(&head->srcu);
 	nvme_put_subsystem(head->subsys);
 	kfree(head->plids);
+	kfree(head->effects);
 	kfree(head);
 }
 
@@ -4022,6 +4023,7 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ns *ns,
 	__must_hold(&ns->ctrl->subsys->lock)
 {
 	struct nvme_ctrl *ctrl = ns->ctrl;
+	struct nvme_effects_log *effects;
 	struct nvme_ns_head *head;
 	size_t size = sizeof(*head);
 	int ret = -ENOMEM;
@@ -4052,11 +4054,22 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ns *ns,
 	ns->head = head;
 
 	if (head->ids.csi) {
-		ret = nvme_get_effects_log(ctrl, head->ids.csi, &head->effects);
+		ret = nvme_get_effects_log(ctrl, head->ids.csi, &effects);
 		if (ret)
 			goto out_cleanup_srcu;
 	} else
-		head->effects = ctrl->effects;
+		effects = ctrl->effects;
+
+	/*
+	 * The effects log belongs to the controller and is freed with it, but
+	 * the head can outlive this controller and be shared with others, so
+	 * keep a private copy.
+	 */
+	head->effects = kmemdup(effects, sizeof(*effects), GFP_KERNEL);
+	if (!head->effects) {
+		ret = -ENOMEM;
+		goto out_cleanup_srcu;
+	}
 
 	if (ctrl->ctratt & NVME_CTRL_ATTR_FDPS) {
 		ret = nvme_query_fdp_info(ns, info);
@@ -4076,6 +4089,7 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ns *ns,
 out_cleanup_fdp:
 	kfree(head->plids);
 out_cleanup_srcu:
+	kfree(head->effects);
 	cleanup_srcu_struct(&head->srcu);
 out_ida_remove:
 	ida_free(&ctrl->subsys->ns_ida, head->instance);
-- 
2.55.0.windows.5