[RFC PATCH] nvme: refuse an unsolicited format change on a namespace that is in use

[Chao Shi]

A namespace can report a new LBA format or metadata size on revalidation.
The host still holds cached data, queued bios and integrity buffers built
for the old geometry.  Adopting the new one reinterprets all of it.

Freezing the queue does not help.  The request is already complete when
the integrity verify is handed to kintegrityd, so the freeze drains while
that work is still pending.  The verify then walks a buffer sized for the
old metadata_size using the new step size.  That is the reported KASAN
slab-out-of-bounds read in t10_pi_verify().

Christoph suggested failing such a revalidation rather than handling the
fallout.  Do that.  Compare the new LBA data size and metadata size
against the live ones, before the queue is frozen.  If they differ while
the disk is open, return INVALID_NS/DNR.  nvme_validate_ns() turns that
into nvme_ns_remove(), as it already does for changed identifiers.

Host-initiated format and namespace management are exempt.  They arrive
through nvme_passthru_end(), where the host asked for the change.
NVME_CTRL_SELF_RESCAN marks that window.

Link: https://lore.kernel.org/linux-block/ah03bXpgFLQjOUt8@infradead.org/
Link: https://lore.kernel.org/linux-block/20260531-blk-integrity-fix-v1-1-cc7084f42cf1@outlook.com/
Found by FuzzNvme.

Signed-off-by: Chao Shi <coshi036 at gmail.com>
---
RFC: the refusal path is untested and the scoping is the part I am least
sure of.

* Build-tested only.  W=1 clean, with and without CONFIG_NVME_MULTIPATH.
  syzkaller could not extract a reproducer, so the new path has not run.

* Is the exemption right?  It keeps "nvme format" working on a namespace
  that merely has a udev probe open.  But it also lets through the one
  case where a change is expected.

* NVME_CTRL_SELF_RESCAN is best effort.  An AER-driven rescan inside the
  nvme_passthru_end() window looks solicited.

* Only lba_shift and ms are compared.  A PI type change with the same
  metadata size is not caught.

* Replaces an earlier attempt from my group that Christoph NAK'd.  We are
  dropping it rather than respinning it.

 drivers/nvme/host/core.c | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 453c1f0b2dd0..ebac5a3d1a07 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1285,8 +1285,15 @@ void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 		}
 	}
 	if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)) {
+		/*
+		 * The host asked for this change, so let the rescan adopt the
+		 * new namespace geometry even if the disk is open.  Only an
+		 * unsolicited change is refused, see nvme_update_ns_info_block().
+		 */
+		set_bit(NVME_CTRL_SELF_RESCAN, &ctrl->flags);
 		nvme_queue_scan(ctrl);
 		flush_work(&ctrl->scan_work);
+		clear_bit(NVME_CTRL_SELF_RESCAN, &ctrl->flags);
 	}
 	if (ns)
 		return;
@@ -2384,6 +2391,17 @@ static bool nvme_invalid_lba_sz(u64 nsze, signed int shift, sector_t *capacity)
 	return check_shl_overflow(nsze, shift, capacity);
 }
 
+/*
+ * Openers of a multipath namespace go to the head disk; the per-path disk is
+ * hidden and never has any.
+ */
+static unsigned int nvme_ns_openers(struct nvme_ns *ns)
+{
+	if (nvme_ns_head_multipath(ns->head))
+		return disk_openers(ns->head->disk);
+	return disk_openers(ns->disk);
+}
+
 static int nvme_update_ns_info_block(struct nvme_ns *ns,
 		struct nvme_ns_info *info)
 {
@@ -2436,6 +2454,28 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
 		goto out;
 	}
 
+	/*
+	 * Changing the LBA format or the metadata size reinterprets everything
+	 * the host has already cached, queued or handed to the integrity code
+	 * for this namespace, and freezing the queue does not cover any of it:
+	 * page cache contents, bios batched on a plug and the deferred
+	 * integrity verify work all outlive the freeze.  If such a change
+	 * arrives unsolicited while the namespace is in use, refuse it and let
+	 * the caller take the namespace offline rather than adopt a geometry
+	 * that describes something else than what the host is holding.
+	 */
+	if (nvme_ns_openers(ns) &&
+	    !test_bit(NVME_CTRL_SELF_RESCAN, &ns->ctrl->flags) &&
+	    (ns->head->lba_shift != id->lbaf[lbaf].ds ||
+	     ns->head->ms != le16_to_cpu(id->lbaf[lbaf].ms))) {
+		dev_err(ns->ctrl->device,
+			"unsolicited format change on in-use nsid %u (lba_shift %u -> %u, ms %u -> %u)\n",
+			info->nsid, ns->head->lba_shift, id->lbaf[lbaf].ds,
+			ns->head->ms, le16_to_cpu(id->lbaf[lbaf].ms));
+		ret = NVME_SC_INVALID_NS | NVME_STATUS_DNR;
+		goto out;
+	}
+
 	lim = queue_limits_start_update(ns->disk->queue);
 
 	memflags = blk_mq_freeze_queue(ns->disk->queue);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 824651cc898d..93c15d5580e3 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -329,6 +329,7 @@ enum nvme_ctrl_flags {
 	NVME_CTRL_SKIP_ID_CNS_CS	= 4,
 	NVME_CTRL_DIRTY_CAPABILITY	= 5,
 	NVME_CTRL_FROZEN		= 6,
+	NVME_CTRL_SELF_RESCAN		= 7,
 };
 
 struct nvme_ctrl {

base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
-- 
2.43.0