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

[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@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

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

[Keith Busch]

On Tue, Aug 11, 2026 at 03:21:11PM -0400, Chao Shi wrote:
> @@ -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;
> +	}

Refusing to acknowledge the new format doesn't mean you get to continue
using the old format. You're going to corrupt memory and data this way.

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

[Chris S]

On Tue, Aug 11, 2026 at 01:30:54PM -0600, Keith Busch wrote:
> Refusing to acknowledge the new format doesn't mean you get to continue
> using the old format. You're going to corrupt memory and data this way.

You're right - the teardown is the problem, not the refusal.

nvme_ns_remove() never sets GD_DEAD, so del_gendisk() takes the
non-surprise path and asks the filesystem to write back.  Those bios
go through nvme_setup_rw() with the stale lba_shift.  set_capacity(0)
doesn't catch them on a partition, which keeps its own bd_nr_sectors.

So it should mark the disk dead first and drop the cache instead:

        blk_mark_disk_dead(ns->disk);
        ret = NVME_SC_INVALID_NS | NVME_STATUS_DNR;

Multipath needs more than that - the openers are on head->disk, and
only the path gets removed.

Is that the direction, or is refuse-and-remove wrong here to begin
with?

Best,
Chao

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

[Keith Busch]

On Tue, Aug 11, 2026 at 04:29:51PM -0400, Chris S wrote:
> On Tue, Aug 11, 2026 at 01:30:54PM -0600, Keith Busch wrote:
> > Refusing to acknowledge the new format doesn't mean you get to continue
> > using the old format. You're going to corrupt memory and data this way.
> 
> You're right - the teardown is the problem, not the refusal.
> 
> nvme_ns_remove() never sets GD_DEAD, so del_gendisk() takes the
> non-surprise path and asks the filesystem to write back.  Those bios
> go through nvme_setup_rw() with the stale lba_shift.  set_capacity(0)
> doesn't catch them on a partition, which keeps its own bd_nr_sectors.
> 
> So it should mark the disk dead first and drop the cache instead:
> 
>         blk_mark_disk_dead(ns->disk);
>         ret = NVME_SC_INVALID_NS | NVME_STATUS_DNR;
> 
> Multipath needs more than that - the openers are on head->disk, and
> only the path gets removed.
> 
> Is that the direction, or is refuse-and-remove wrong here to begin
> with?

You need a nvme spec level change for this to work. There's nothing we
can do today to close every race where the driver attempts to use a
stale LBA shift when the format was changed externally.

What I proposed in the past was that the device has to return a new
error code for all new IO until the host queries the appropriate
Namespace identification to acknowledge the new LBA format. Kind of like
a SCSI "Check Condition Parameters Changed". An enlightend host could
trigger a queue freeze and namespace rescan after seeing this status,
clearing the device's latch and resume IO after setting up the new queue
limits.