[PATCH] nvme: fix command effects log lifetime for multipath heads

Yao Sang <[email protected]>
Newsgroups org.infradead.lists.linux-nvme,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
KASAN reported a use-after-free when an I/O passthrough command was sent
through a multipath namespace head after the controller path that first
created the head had been removed:

  BUG: KASAN: slab-use-after-free in nvme_command_effects+0x192/0x200 [nvme_core]
  Read of size 4 at addr ffff888141b14400 by task nvme/19811
  nvme_command_effects+0x192/0x200 [nvme_core]
  nvme_cmd_allowed+0x7e/0x1b0 [nvme_core]
  nvme_user_cmd.constprop.0+0x1b5/0x450 [nvme_core]
  nvme_ns_head_chr_ioctl+0xf4/0x2a0 [nvme_core]

The report showed the command effects log was allocated from
nvme_get_effects_log() during controller probe and freed from
nvme_free_ctrl() when that controller was removed.

The command effects logs are cached in ctrl->cels and freed with the
controller.  A namespace head, however, is shared by all paths to the
same namespace.  Storing the first path's log pointer in the namespace
head leaves the head pointing at controller-owned memory after that path
is removed, even though another path still keeps the namespace head alive.

Keep the command effects log pointer in struct nvme_ns instead. The
multipath head ioctl path already selects a live namespace path before
checking command effects, so use the log owned by that path's controller.

Fixes: be93e87e7802 ("nvme: support for multiple Command Sets Supported and Effects log pages")
Signed-off-by: Yao Sang <[email protected]>
---
A blktests regression test has been posted as nvme/070:
https://lore.kernel.org/r/[email protected]

 drivers/nvme/host/core.c | 15 +++++++--------
 drivers/nvme/host/nvme.h |  3 ++-
 drivers/nvme/host/zns.c  |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1322c678f4eb..7d062236e2bd 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1236,7 +1236,7 @@ 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]);
+		effects = le32_to_cpu(ns->effects->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",
@@ -4033,13 +4033,6 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
 	ratelimit_set_flags(&head->rs_nuse, RATELIMIT_MSG_ON_RELEASE);
 	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;
@@ -4132,6 +4125,12 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 	}
 
 	mutex_lock(&ctrl->subsys->lock);
+	ret = nvme_get_effects_log(ctrl, info->ids.csi, &ns->effects);
+	if (ret) {
+		if (ret > 0)
+			ret = blk_status_to_errno(nvme_error_status(ret));
+		goto out_unlock;
+	}
 	head = nvme_find_ns_head(ctrl, info->nsid);
 	if (!head) {
 		ret = nvme_subsys_check_duplicate_ids(ctrl->subsys, &info->ids);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 75e5d5a8a77c..54044b15af90 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -553,7 +553,6 @@ struct nvme_ns_head {
 	bool			shared;
 	bool			rotational;
 	bool			passthru_err_log_enabled;
-	struct nvme_effects_log *effects;
 	u64			nuse;
 	unsigned		ns_id;
 	int			instance;
@@ -618,6 +617,8 @@ struct nvme_ns {
 	struct list_head siblings;
 	struct kref kref;
 	struct nvme_ns_head *head;
+	/* Command effects log for this controller path. */
+	struct nvme_effects_log *effects;
 
 	unsigned long flags;
 #define NVME_NS_REMOVING		0
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 2a152e87bd76..29130b3697d1 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -38,7 +38,7 @@ static int nvme_set_max_append(struct nvme_ctrl *ctrl)
 int nvme_query_zone_info(struct nvme_ns *ns, unsigned lbaf,
 		struct nvme_zone_info *zi)
 {
-	struct nvme_effects_log *log = ns->head->effects;
+	struct nvme_effects_log *log = ns->effects;
 	struct nvme_command c = { };
 	struct nvme_id_ns_zns *id;
 	int status;
-- 
2.25.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.