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

Yao Sang posted 1 patch 1 week, 4 days ago
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(-)
[PATCH] nvme: fix command effects log lifetime for multipath heads
Posted by Yao Sang 1 week, 4 days ago
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 <sangyao@kylinos.cn>
---
A blktests regression test has been posted as nvme/070:
https://lore.kernel.org/r/20260814070019.3860097-1-sangyao@kylinos.cn

 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