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

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

[shao.mingyin]

This series fixes two issues in the nvmet Identify and Get Log Page
paths.  Both patches are independent and can be applied on their own or
as a subset.

Patch 1 zeroes the unused part of the fixed-size Get Log Page replies.
The transfer length is supplied by the host, and the transports send the
whole buffer back, so a request larger than the log page used to return
stale kernel memory.  It covers the Supported Log Pages, Commands
Supported and Effects, Supported Features, Endurance Group, Reservation
and Rotational Media Information pages, and clamps the Error log ring
walk to the requested length while keeping the most recent error first,
as the specification requires.

Patch 2 returns the namespace lookup error for an Identify command with
CNS 05h and CSI 02h on a namespace that does not exist, instead of
overwriting it with the status of the identify buffer copy.

Both patches were runtime-tested on 7.3.0-next-20261030 (QEMU/TCG
guest).  Patch 1 was tested with an nvmet-tcp target and a file-backed
namespace, using a 4096-byte Get Log Page; patch 2 with the loop
transport and CONFIG_BLK_DEV_ZONED.  The Rotational Media Information
and Reservation log changes of patch 1 are not runtime-tested: the test
guest has no rotational namespace and no persistent-reservation setup.

Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

[PATCH 1/2] nvmet: zero out unused data of fixed-size log pages

[shao.mingyin]

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

Several Get Log Page handlers copy a fixed-size structure to the host
buffer and return success, without initializing the rest of the buffer
the host asked for.  The requested data length (NUMD) is supplied by the
host, so it can be larger than the structure being returned.

The transports allocate those buffers with sgl_alloc() or kmalloc(),
none of them with __GFP_ZERO, and on success they send the whole
transfer length back to the host.  A host requesting more than the size
of the log page therefore receives stale kernel memory from the target.
This is the same class of bug as the one fixed for Zone Management
Receive in commit 31b8a6af19b7 ("nvmet: zns: don't leak stale memory in
zone management receive").

Zero the unused tail after filling the Supported Log Pages, Commands
Supported and Effects, Supported Features, Endurance Group, Reservation
and Rotational Media Information log pages.

The Rotational Media Information log used to reject a host buffer that
was larger than the fixed structure.  Accept that and zero the tail
instead, like the other handlers.

The Error log page needs more than a zeroed tail.  Clamp the ring walk
to the requested length, zero the remaining bytes, and propagate
nvmet_copy_to_sgl() failures instead of completing with success.  The
ring is walked backwards from the slot of the last error because the
NVMe base specification requires the most recent error to be the first
log entry.

The Reservation log tail is zeroed after the log_mgr lock is dropped:
the host controls the length, so the buffer can be large.

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>

---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with an nvmet-tcp target
and a file-backed namespace; the target's data pages are primed with
0xaa by a preceding 1 MiB write.

A Get Log Page for Supported Log Pages with a 4096-byte transfer
(NUMD 1023) asks for four times sizeof(struct nvme_supported_log), so
bytes 1024..4095 are never written by the handler:

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

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 | 53 +++++++++++++++++++++++++++++----
 drivers/nvme/target/pr.c        |  7 +++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 26028be8fe4d..434fa543019a 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -176,16 +176,29 @@ 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;
+
+	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 +208,20 @@ 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);
+
+	/*
+	 * Zero what the ring walk did not fill: either the tail beyond the
+	 * fixed log, or the partial entry at the end of a short request.
+	 */
+	if (req->transfer_len > offset) {
+		u16 zero_status;
+
+		zero_status = nvmet_zero_sgl(req, offset,
+					     req->transfer_len - offset);
+		if (!status)
+			status = zero_status;
+	}
+	nvmet_req_complete(req, status);
 }
 
 static void nvmet_execute_get_supported_log_pages(struct nvmet_req *req)
@@ -222,6 +248,9 @@ static void nvmet_execute_get_supported_log_pages(struct nvmet_req *req)
 	logs->lids[NVME_LOG_RESERVATION] = cpu_to_le32(NVME_LIDS_LSUPP);
 
 	status = nvmet_copy_to_sgl(req, 0, logs, sizeof(*logs));
+	if (!status && req->transfer_len > sizeof(*logs))
+		status = nvmet_zero_sgl(req, sizeof(*logs),
+					req->transfer_len - sizeof(*logs));
 	kfree(logs);
 out:
 	nvmet_req_complete(req, status);
@@ -303,7 +332,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;
 	}
@@ -322,6 +351,9 @@ static void nvmet_execute_get_log_page_rmi(struct nvmet_req *req)
 		log->numa = cpu_to_le16(1);
 
 	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	if (!status && req->transfer_len > sizeof(*log))
+		status = nvmet_zero_sgl(req, sizeof(*log),
+					req->transfer_len - sizeof(*log));
 	kfree(log);
 out:
 	nvmet_req_complete(req, status);
@@ -437,6 +469,9 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
 	}
 
 	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	if (!status && req->transfer_len > sizeof(*log))
+		status = nvmet_zero_sgl(req, sizeof(*log),
+					req->transfer_len - sizeof(*log));
 free:
 	kfree(log);
 out:
@@ -528,6 +563,9 @@ static void nvmet_execute_get_log_page_endgrp(struct nvmet_req *req)
 	put_unaligned_le64(data_units_written, &log->duw[0]);
 copy:
 	status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));
+	if (!status && req->transfer_len > sizeof(*log))
+		status = nvmet_zero_sgl(req, sizeof(*log),
+					req->transfer_len - sizeof(*log));
 	kfree(log);
 out:
 	nvmet_req_complete(req, status);
@@ -602,6 +640,9 @@ static void nvmet_execute_get_log_page_features(struct nvmet_req *req)
 		cpu_to_le32(NVME_FIS_FSUPP | NVME_FIS_NSCPE);
 
 	status = nvmet_copy_to_sgl(req, 0, features, sizeof(*features));
+	if (!status && req->transfer_len > sizeof(*features))
+		status = nvmet_zero_sgl(req, sizeof(*features),
+					req->transfer_len - 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..99fdcbf572a9 100644
--- a/drivers/nvme/target/pr.c
+++ b/drivers/nvme/target/pr.c
@@ -130,6 +130,13 @@ void nvmet_execute_get_log_page_resv(struct nvmet_req *req)
 out:
 	status = nvmet_copy_to_sgl(req, 0, &log, sizeof(log));
 	mutex_unlock(&log_mgr->lock);
+	/*
+	 * Zero the unused tail outside the log_mgr lock: the host controls
+	 * the length, so this can touch a large buffer.
+	 */
+	if (!status && req->transfer_len > sizeof(log))
+		status = nvmet_zero_sgl(req, sizeof(log),
+					req->transfer_len - sizeof(log));
 	nvmet_req_complete(req, status);
 }
 
-- 
2.27.0

Re: [PATCH 1/2] nvmet: zero out unused data of fixed-size log pages

[Christoph Hellwig]

Given none of these are in a fast path, should we simply zero the
entire transfer length first?

Also please cut down the commit messages a bit to be on-point.

Re: [PATCH 1/2] nvmet: zero out unused data of fixed-size log pages

[shao.mingyin]

On Fri, Nov 06, 2026 at 11:39:32AM +0100, Christoph Hellwig wrote:
> Given none of these are in a fast path, should we simply zero the
> entire transfer length first?

Yes.  v2 does that: each handler zeroes the whole transfer length and
then copies the fixed structure over it, instead of zeroing the unused
tail afterwards.  The Error log zeroes first as well and clamps the ring
walk to the requested length.

> Also please cut down the commit messages a bit to be on-point.

I also cut both commit messages down; the long rationale is gone.

v2 follows.

Thanks,
Shao

[PATCH 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 the done label when
nvmet_req_find_ns() fails, and that label unconditionally overwrites
status with the result of nvmet_copy_to_sgl():

	status = nvmet_req_find_ns(req);
	if (status)
		goto done;
	...
done:
	status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));

The identify command has already been checked to transfer exactly
NVME_IDENTIFY_DATA_SIZE bytes, so copying the zeroed id_zns always
succeeds.  An Identify command with CNS 05h and CSI 02h for a namespace
that does not exist therefore completes successfully with a zeroed
identify structure instead of returning Invalid Namespace, hiding the
error from the host.

Return the lookup error directly.  The other error paths in this
function already use the out label.

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 2/2] nvmet: zns: return the namespace lookup error for Identify

[Christoph Hellwig]

The fix looks good, but please write to the point commit logs instead
of detective novels.

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

[shao.mingyin]

On Fri, Nov 06, 2026 at 11:40:12AM +0100, Christoph Hellwig wrote:
> The fix looks good, but please write to the point commit logs instead
> of detective novels.

Fair.  v2 keeps the code unchanged and rewrites the commit message to two
short paragraphs: the done label overwrites the lookup error, and the
fix returns it.

v2 follows.

Thanks,
Shao