[PATCH v2 0/2] nvmet: fix stale log page data and a ZNS Identify error

[PATCH v2 0/2] nvmet: fix stale log page data and a ZNS Identify error

[shao.mingyin]

This is v2 of the two-patch series.  Patch 1 now follows Christoph's
suggestion to zero the whole transfer buffer instead of only the unused
tail, and both commit messages are shorter.

Patch 1 zeroes the entire Get Log Page transfer buffer before filling
it, so a host that requests more bytes than the fixed structure no
longer receives stale kernel memory.  The Error log also clamps its
ring walk to the requested length instead of relying on a zeroed tail.
Patch 2 returns the namespace lookup error for a ZNS Identify on a
missing namespace instead of overwriting it with a successful copy.

The two patches are independent and can be applied in any order.  Both
carry Fixes: and Cc: stable.

Changes in v2:
 - 1/2: zero the whole transfer length first instead of only the unused
   tail, as suggested by Christoph.
 - 1/2: shorten the commit message.
 - 2/2: shorten the commit message; no code change.

Link: https://lore.kernel.org/all/20261105215753914RuQA4AVkFtXzpBkieEscY@zte.com.cn/

Testing: the uninitialized tail was made visible with page_poison=1
(CONFIG_PAGE_POISONING).  On the unpatched kernel a 4096-byte Supported
Log Pages request returned 0xaa in bytes 1024..4095 (10/10); with patch 1
the tail is zero (0/10).  The other log-page changes and the error-log
ring clamp are not runtime-tested.  Patch 2 is unchanged from v1, where
Identify CNS 05h/CSI 02h for a missing NSID returned Invalid Namespace
after the fix.

Shao Mingyin (2):
  nvmet: zero the entire Get Log Page transfer buffer
  nvmet: zns: return the namespace lookup error for Identify

 drivers/nvme/target/admin-cmd.c | 51 ++++++++++++++++++++++++++++++++---------
 drivers/nvme/target/pr.c        |  6 +++++
 drivers/nvme/target/zns.c       |  3 +--
 3 files changed, 47 insertions(+), 13 deletions(-)

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

[PATCH v2 1/2] nvmet: zero the entire Get Log Page transfer buffer

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

Several fixed-size Get Log Page handlers copy only the log structure
and leave the rest of the host buffer untouched.  The transport buffers
are not zeroed, so a host requesting more bytes than the structure
receives stale kernel memory.

Zero the entire transfer length before filling the buffer in the
Supported Log Pages, Commands Supported and Effects, Supported
Features, Endurance Group, Rotational Media Information and Reservation
handlers.  For the Error log, also clamp the ring walk to the requested
length and propagate copy failures instead of completing with success.

The Rotational Media Information handler now accepts a buffer larger
than its fixed structure.

Fixes: 11ad507784ed ("nvmet: add error log page cmd handler")
Fixes: 83acb24e6de7 ("nvmet: implement supported log pages")
Fixes: 0866bf0c3778 ("nvmet: add commands supported and effects log page")
Fixes: e973c91727d4 ("nvmet: implement supported features log")
Fixes: 266b652c65b4 ("nvmet: implement endurance groups")
Fixes: 5fd075cdaf36 ("nvmet: implement rotational media information log")
Fixes: 5a47c2080a73 ("nvmet: support reservation feature")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

---
The uninitialized tail is made visible with page_poison=1
(CONFIG_PAGE_POISONING): freed pages are filled with 0xaa, so a
Supported Log Pages request that asks for 4096 bytes (NUMD 1023) shows
bytes 1024..4095 as 0xaa on the unpatched kernel.  The request is four
times sizeof(struct nvme_supported_log), so the handler writes only the
first 1024 bytes.

  unpatched                            patched
  tail[1024..4095] = 0xaa (10/10)      tail zeroed (0/10)

Without page poisoning the same bytes would carry whatever stale data
the target's page allocation returns; the patch zeroes them first.

The Rotational Media Information and Reservation log changes, and the
error log ring clamp, are not runtime-tested: the test guest has no
rotational namespace, no persistent-reservation setup, and no injected
command error.
---
 drivers/nvme/target/admin-cmd.c | 51 ++++++++++++++++++++++++++++++++---------
 drivers/nvme/target/pr.c        |  6 +++++
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 26028be8fe4d..6cb05d33f024 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -176,16 +176,35 @@ static void nvmet_execute_get_log_page_error(struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	unsigned long flags;
-	off_t offset = 0;
+	u32 offset = 0;
+	u32 len;
 	u64 slot;
 	u64 i;
+	u16 status = 0;
+
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (status) {
+		nvmet_req_complete(req, status);
+		return;
+	}
+
+	len = min_t(u32, req->transfer_len,
+		    NVMET_ERROR_LOG_SLOTS * sizeof(struct nvme_error_slot));
 
 	spin_lock_irqsave(&ctrl->error_lock, flags);
+
+	/*
+	 * The NVMe base specification requires the most recent error to be
+	 * the first log entry, so walk the ring backwards from the slot of
+	 * the last error and stop at the end of the requested buffer.
+	 */
 	slot = ctrl->err_counter % NVMET_ERROR_LOG_SLOTS;
 
-	for (i = 0; i < NVMET_ERROR_LOG_SLOTS; i++) {
-		if (nvmet_copy_to_sgl(req, offset, &ctrl->slots[slot],
-				sizeof(struct nvme_error_slot)))
+	for (i = 0; i < NVMET_ERROR_LOG_SLOTS &&
+	     offset + sizeof(struct nvme_error_slot) <= len; i++) {
+		status = nvmet_copy_to_sgl(req, offset, &ctrl->slots[slot],
+					   sizeof(struct nvme_error_slot));
+		if (status)
 			break;
 
 		if (slot == 0)
@@ -195,7 +214,7 @@ static void nvmet_execute_get_log_page_error(struct nvmet_req *req)
 		offset += sizeof(struct nvme_error_slot);
 	}
 	spin_unlock_irqrestore(&ctrl->error_lock, flags);
-	nvmet_req_complete(req, 0);
+	nvmet_req_complete(req, status);
 }
 
 static void nvmet_execute_get_supported_log_pages(struct nvmet_req *req)
@@ -221,7 +240,9 @@ static void nvmet_execute_get_supported_log_pages(struct nvmet_req *req)
 	logs->lids[NVME_LOG_RMI] = cpu_to_le32(NVME_LIDS_LSUPP);
 	logs->lids[NVME_LOG_RESERVATION] = cpu_to_le32(NVME_LIDS_LSUPP);
 
-	status = nvmet_copy_to_sgl(req, 0, logs, sizeof(*logs));
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (!status)
+		status = nvmet_copy_to_sgl(req, 0, logs, sizeof(*logs));
 	kfree(logs);
 out:
 	nvmet_req_complete(req, status);
@@ -303,7 +324,7 @@ static void nvmet_execute_get_log_page_rmi(struct nvmet_req *req)
 		goto out;
 	}
 
-	if (req->transfer_len != sizeof(*log)) {
+	if (req->transfer_len < sizeof(*log)) {
 		status = NVME_SC_SGL_INVALID_DATA | NVME_STATUS_DNR;
 		goto out;
 	}
@@ -321,7 +342,9 @@ static void nvmet_execute_get_log_page_rmi(struct nvmet_req *req)
 	else
 		log->numa = cpu_to_le16(1);
 
-	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (!status)
+		status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
 	kfree(log);
 out:
 	nvmet_req_complete(req, status);
@@ -436,7 +459,9 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
 		goto free;
 	}
 
-	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (!status)
+		status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
 free:
 	kfree(log);
 out:
@@ -527,7 +552,9 @@ static void nvmet_execute_get_log_page_endgrp(struct nvmet_req *req)
 	put_unaligned_le64(host_writes, &log->hwc[0]);
 	put_unaligned_le64(data_units_written, &log->duw[0]);
 copy:
-	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (!status)
+		status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
 	kfree(log);
 out:
 	nvmet_req_complete(req, status);
@@ -601,7 +628,9 @@ static void nvmet_execute_get_log_page_features(struct nvmet_req *req)
 	features->fis[NVME_FEAT_RESV_MASK] =
 		cpu_to_le32(NVME_FIS_FSUPP | NVME_FIS_NSCPE);
 
-	status = nvmet_copy_to_sgl(req, 0, features, sizeof(*features));
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (!status)
+		status = nvmet_copy_to_sgl(req, 0, features, sizeof(*features));
 	kfree(features);
 out:
 	nvmet_req_complete(req, status);
diff --git a/drivers/nvme/target/pr.c b/drivers/nvme/target/pr.c
index 14eaa99f98fa..b860cbe192cd 100644
--- a/drivers/nvme/target/pr.c
+++ b/drivers/nvme/target/pr.c
@@ -99,6 +99,12 @@ void nvmet_execute_get_log_page_resv(struct nvmet_req *req)
 	u64 cur_count;
 	u64 next_count;
 
+	status = nvmet_zero_sgl(req, 0, req->transfer_len);
+	if (status) {
+		nvmet_req_complete(req, status);
+		return;
+	}
+
 	mutex_lock(&log_mgr->lock);
 	if (!kfifo_get(&log_mgr->log_queue, &log))
 		goto out;

[PATCH v2 2/2] nvmet: zns: return the namespace lookup error for Identify

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvmet_execute_identify_ns_zns() jumps to done when nvmet_req_find_ns()
fails, and that label overwrites status with a successful copy of the
zeroed identify data.  An Identify with CNS 05h and CSI 02h for a
missing NSID therefore completes with SUCCESS and zeroed data instead
of Invalid Namespace.

Return the lookup error instead.

Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the loop transport
and CONFIG_BLK_DEV_ZONED.

An Identify with CNS 05h and CSI 02h for a namespace that does not
exist (NSID 100) completes successfully and returns a zeroed structure:

  unpatched                            patched
  ret=0, all 4096 bytes zero           ret=24587 (Invalid Namespace)

diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 92f5c1435582..cea05b4bc740 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -108,7 +108,7 @@ void nvmet_execute_identify_ns_zns(struct nvmet_req *req)
 
 	status = nvmet_req_find_ns(req);
 	if (status)
-		goto done;
+		goto out;
 
 	if (nvmet_ns_revalidate(req->ns)) {
 		mutex_lock(&req->ns->subsys->lock);
@@ -140,7 +140,6 @@ void nvmet_execute_identify_ns_zns(struct nvmet_req *req)
 		mar--;
 	id_zns->mar = cpu_to_le32(mar);
 
-done:
 	status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
 out:
 	kfree(id_zns);

Re: [PATCH v2 0/2] nvmet: fix stale log page data and a ZNS Identify error

[Keith Busch]

On Fri, 6 Nov 2026 23:00:13 +0800 (CST), shao.mingyin@zte.com.cn wrote:
> This is v2 of the two-patch series.  Patch 1 now follows Christoph's
> suggestion to zero the whole transfer buffer instead of only the unused
> tail, and both commit messages are shorter.
> 
> Patch 1 zeroes the entire Get Log Page transfer buffer before filling
> it, so a host that requests more bytes than the fixed structure no
> longer receives stale kernel memory.  The Error log also clamps its
> ring walk to the requested length instead of relying on a zeroed tail.
> Patch 2 returns the namespace lookup error for a ZNS Identify on a
> missing namespace instead of overwriting it with a successful copy.
> 
> [...]

Thanks, applied to nvme-7.4.

[1/2] nvmet: zero the entire Get Log Page transfer buffer
      commit: ad606ae5635b68e324005aaa06421be752619dab
[2/2] nvmet: zns: return the namespace lookup error for Identify
      commit: 687f3014afcd5826fa3d8f40b445f647b5a379b6
--
Keith Busch <kbusch@kernel.org>

Re: [PATCH v2 1/2] nvmet: zero the entire Get Log Page transfer buffer

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 2/2] nvmet: zns: return the namespace lookup error for Identify

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>