From: Shao Mingyin 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 --- 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 From: Shao Mingyin 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 --- 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);