[PATCH 0/5] nvme: validate and decode controller supplied values

[PATCH 0/5] nvme: validate and decode controller supplied values

[shao.mingyin]

This series adds missing validation and endian decoding for values that
come from a controller, a backing controller or a user request.

Patch 1 rejects an invalid ZASL from the controller.  nvme_set_max_append()
shifts 1 by zasl + 3; a value of 29 or more shifts a 32-bit value by 32 or
more, and the old code also used a signed int for the shift.

Patch 2 maps the user data buffer whenever the ioctl length is non-zero,
not only when the user address is non-zero.  With addr 0 and a non-zero
length the command was submitted without a data buffer, and the target
rejected it with an invalid SGL status instead of the host rejecting the
user buffer.

Patch 3 rejects an Identify Namespace from a passthrough backing
controller whose nlbaf does not fit the 64-entry LBA format table.  The
old loop cleared fields past the table and returned an inconsistent
Identify structure.

Patch 4 decodes the FDP Get Features result as the little-endian dword
the controller returns, instead of passing a four-byte structure to the
u32 result pointer and reading it in host byte order.

Patch 5 bounds the reservation report retry loop.  A target that keeps
changing the registrant count made the ioctl loop forever, allocating a
full status buffer on every iteration.

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

Testing: all apply to next-20261105, pass checkpatch.pl --strict with
0 errors/0 warnings/0 checks, and zns.c, ioctl.c, passthru.c, core.c
and pr.c build with the corresponding options under W=1.  Each patch's
--- notes describe its QEMU/TCG A/B test with a test-only target hook;
no hardware was used.

Shao Mingyin (5):
  nvme: zns: reject an out-of-range zone append size limit
  nvme: map passthrough data when the length is nonzero
  nvmet: validate the LBA format count of a passthrough namespace
  nvme: decode the FDP Get Features result as a dword
  nvme: bound the reservation report retry loop

 drivers/nvme/host/core.c       |  6 +++---
 drivers/nvme/host/ioctl.c      |  2 +-
 drivers/nvme/host/pr.c         |  4 +++-
 drivers/nvme/host/zns.c        | 11 ++++++++---
 drivers/nvme/target/passthru.c |  5 +++++
 5 files changed, 20 insertions(+), 8 deletions(-)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

[PATCH 1/5] nvme: zns: reject an out-of-range zone append size limit

[shao.mingyin]

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

nvme_set_max_append() computes the zone append limit as

	ctrl->max_zone_append = 1 << (id->zasl + 3);

where zasl is a u8 from the controller's Identify Controller ZNS data
structure.  A value of 29 or more shifts a 32-bit int by 32 or more,
which is undefined behaviour even with the kernel's -fno-strict-overflow;
GCC may turn the result into zero, one, or another arbitrary value.  The
limit then either disables zone append or advertises a size larger than
the controller and the block layer expect.  This is reachable with a
malicious or buggy controller.

Use an unsigned shift, and fall back to max_hw_sectors, as for a zero
ZASL, when the reported value does not fit.  Warn once about the invalid
value so the device can be identified.

Fixes: 240e6ee272c0 ("nvme: support for zoned namespaces")
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 a host-managed
zoned null_blk exported through nvmet and connected over the loop
transport, so the host driver and the target run in the same guest.
A TEST-ONLY hook makes the target report ZASL = 29 in its Identify
Controller ZNS data; only the nvme host patch differs between the two
runs.

The out-of-range shift silently collapses the zone append limit to one
sector on the unpatched driver; the fix falls back to max_hw_sectors
and warns once:

  unpatched                            patched
  zone_append_max_bytes 512            zone_append_max_bytes 1044480
  (max_zone_append = 1 sector)         (fallback to max_hw_sectors)
  no warning                           "invalid zone append size limit 29"

 drivers/nvme/host/zns.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index e31ec6f4f94f..c34015e68f81 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -27,10 +27,15 @@ static int nvme_set_max_append(struct nvme_ctrl *ctrl)
 		return status;
 	}
 
-	if (id->zasl)
-		ctrl->max_zone_append = 1 << (id->zasl + 3);
-	else
+	if (id->zasl && id->zasl + 3 < 32) {
+		ctrl->max_zone_append = 1U << (id->zasl + 3);
+	} else {
+		if (id->zasl)
+			dev_warn(ctrl->device,
+				 "invalid zone append size limit %u\n",
+				 id->zasl);
 		ctrl->max_zone_append = ctrl->max_hw_sectors;
+	}
 	kfree(id);
 	return 0;
 }
-- 
2.43.0

[PATCH 2/5] nvme: map passthrough data when the length is nonzero

[shao.mingyin]

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

nvme_submit_user_cmd() skips mapping when the user address is zero,
even if a nonzero data length was supplied.  Passthrough ioctls and
NVME_IOCTL_SUBMIT_IO can therefore submit a data-transfer command
without a mapped payload.  For example, the loop target returns a
Data SGL Length Invalid status instead of rejecting the user buffer.

Map whenever a length is given and let the mapping helper validate the
address.  This also permits address zero if the process has valid
memory mapped there, matching the io_uring passthrough behavior from
commit 99fde895ff56 ("nvme: map uring_cmd data even if address is 0").

Fixes: d29ec8241c10 ("nvme: submit internal commands through the block layer")
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 a file-backed namespace.

NVME_IOCTL_IO_CMD with addr=0 and a nonzero data_len skips the mapping
on the unpatched kernel and submits the command with an empty data
pointer, so the target rejects it with "Data SGL Length Invalid" (status
0x600f).  With the patch the mapping helper rejects the null user
address with -EFAULT before the command is submitted:

  unpatched                            patched
  control read (valid addr) ret 0      control read (valid addr) ret 0
  read  addr=0 len=4096 ret 0x600f     read  addr=0 len=4096 -EFAULT
  write addr=0 len=4096 ret 0x600f     write addr=0 len=4096 -EFAULT

 drivers/nvme/host/ioctl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index a87c9ca9d06c..22efc5412416 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -207,7 +207,7 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 		return PTR_ERR(req);
 
 	req->timeout = timeout;
-	if (ubuffer && bufflen) {
+	if (bufflen) {
 		ret = nvme_map_user_request(req, ubuffer, bufflen, meta_buffer,
 				meta_len, NULL, flags);
 		if (ret)
-- 
2.43.0

[PATCH 3/5] nvmet: validate the LBA format count of a passthrough namespace

[shao.mingyin]

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

nvmet_passthru_override_id_ns() clears the metadata size of every LBA
format reported by the backing controller:

	for (i = 0; i < (id->nlbaf + 1); i++)
		if (id->lbaf[i].ms)
			memset(&id->lbaf[i], 0, sizeof(id->lbaf[i]));

The Identify Namespace data comes from the backing controller and nlbaf
is a u8 in that structure, while the LBA format table has only 64
entries.  A broken or malicious backing controller can report nlbaf =
0xff, which makes the loop clear the vendor specific fields that follow
the LBA format table and return an inconsistent identify structure to
the host, e.g. a format count that does not match the table.

Reject such an identify data structure and return an internal error
instead of rewriting unrelated fields.

Fixes: c1fef73f793b ("nvmet: add passthru code to process commands")
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.
A passthru subsystem is backed by a file-backed loop namespace, so the
host driver and the target run in the same guest.  A TEST-ONLY hook
makes the backing Identify Namespace report nlbaf = 0xff; only the
nvmet fix differs between the two runs.

Sending Identify Namespace (CNS = 0) to the passthru controller, the
unpatched loop runs (nlbaf + 1) = 256 times against the 64-entry LBA
format table and clears the 768 vendor bytes right after it:

  unpatched                            patched
  identify accepted (ret = 0)          identify rejected (ret = 6,
  nlbaf = 0xff                        NVME_SC_INTERNAL)
  vs[0..767] cleared to 0              no data returned
  vs[768] still 0xaa

 drivers/nvme/target/passthru.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 6b67880c7bb2..30c06bf0e5f1 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -195,6 +195,11 @@ static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
 	if (status)
 		goto out_free;
 
+	if (id->nlbaf >= ARRAY_SIZE(id->lbaf)) {
+		status = NVME_SC_INTERNAL;
+		goto out_free;
+	}
+
 	for (i = 0; i < (id->nlbaf + 1); i++)
 		if (id->lbaf[i].ms)
 			memset(&id->lbaf[i], 0, sizeof(id->lbaf[i]));
-- 
2.43.0

[PATCH 4/5] nvme: decode the FDP Get Features result as a dword

[shao.mingyin]

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

nvme_query_fdp_info() passes a struct nvme_fdp_config, which is exactly
four bytes, as the result buffer of nvme_get_features():

	struct nvme_fdp_config fdp;

	ret = nvme_get_features(ctrl, NVME_FEAT_FDP, info->endgid, NULL, 0,
				&fdp);
	if (!(fdp.flags & FDPCFG_FDPE))
		return 0;
	ret = nvme_query_fdp_granularity(ctrl, info, fdp.fdpcidx);

nvme_features() stores the result with

	*result = le32_to_cpu(res.u32);

i.e. it writes a CPU-endian dword through the pointer.  On a big-endian
host this puts the flags byte in the fourth byte of the structure, so
fdp.flags is always zero and FDP support is silently disabled, while
fdpcidx would be read from the wrong byte.

Read the result as a u32 and decode the flags and the FDP config index
explicitly, which works on both endiannesses.

Fixes: 30b5f20bb2dd ("nvme: register fdp parameters with the block layer")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 under QEMU/TCG (guest s390x, big-endian),
loop transport + file-backed namespace.  The nvmet target needs no FDP
hardware: a TEST-ONLY stub (applied to both kernels) advertises FDPS and
answers the FDP Get Features / configs log / RUHS commands; the only
difference between the two kernels is this patch.  The FDP probe runs
during namespace scan; the verdict is
/sys/block/nvme0n1/queue/max_write_streams (= nr_plids):

  unpatched (big-endian): max_write_streams=0  FDP silently disabled
  patched   (big-endian): max_write_streams=1  FDP registered

On x86 (little-endian) both kernels report max_write_streams=1, i.e. no
regression.

 drivers/nvme/host/core.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a7a4d09644c1..45feafa00325 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2339,8 +2339,8 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
 	struct nvme_ns_head *head = ns->head;
 	struct nvme_ctrl *ctrl = ns->ctrl;
 	struct nvme_fdp_ruh_status *ruhs;
-	struct nvme_fdp_config fdp;
 	struct nvme_command c = {};
+	u32 fdp;
 	size_t size;
 	int i, ret;
 
@@ -2351,10 +2351,10 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
 		return ret;
 	}
 
-	if (!(fdp.flags & FDPCFG_FDPE))
+	if (!(fdp & FDPCFG_FDPE))
 		return 0;
 
-	ret = nvme_query_fdp_granularity(ctrl, info, fdp.fdpcidx);
+	ret = nvme_query_fdp_granularity(ctrl, info, (fdp >> 8) & 0xff);
 	if (!info->runs)
 		return ret;
 
-- 
2.43.0

Re: [PATCH 4/5] nvme: decode the FDP Get Features result as a dword

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] nvme: uninitialized stack memory use in
  nvme_query_fdp_granularity() [1]
--

commit 37e4c911db178216572cd8ee6fd19cfc2f58c7f0
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

nvme: decode the FDP Get Features result as a dword

This commit changes nvme_query_fdp_info() to read the NVME_FEAT_FDP Get
Features result as a u32 and decode the flags and FDP configuration index
manually so that FDP probing works on big-endian hosts.

> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7a4d09644c13..45feafa003257 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2339,8 +2339,8 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>  	struct nvme_ns_head *head = ns->head;
>  	struct nvme_ctrl *ctrl = ns->ctrl;
>  	struct nvme_fdp_ruh_status *ruhs;
> -	struct nvme_fdp_config fdp;
>  	struct nvme_command c = {};
> +	u32 fdp;
>  	size_t size;
>  	int i, ret;
>  
> @@ -2351,10 +2351,10 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>  		return ret;
>  	}
>  
> -	if (!(fdp.flags & FDPCFG_FDPE))
> +	if (!(fdp & FDPCFG_FDPE))
>  		return 0;
>  
> -	ret = nvme_query_fdp_granularity(ctrl, info, fdp.fdpcidx);
> +	ret = nvme_query_fdp_granularity(ctrl, info, (fdp >> 8) & 0xff);

[Severity: Medium]
This problem wasn't introduced by this patch, but can
nvme_query_fdp_granularity() read uninitialized stack memory from hdr.sze
[1]?

drivers/nvme/host/core.c:nvme_query_fdp_granularity() {
	struct nvme_fdp_config_log hdr, *h;
	...
	ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0,
			       NVME_CSI_NVM, &hdr, size, 0, info->endgid);
	if (ret) {
		...
		return ret;
	}

	size = le32_to_cpu(hdr.sze);
	...
	h = kvzalloc(size, GFP_KERNEL);
}

hdr is allocated on the stack without zero-initialization.  If the
controller completes the log page command with a short data transfer,
hdr.sze remains uninitialized and is passed as the allocation size to
kvzalloc().

[1] https://sashiko.dev/#/bug/linux-1cda5d44-a6cc-40a9-bd2b-cbed1b364093

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/202611150127512368BGQ07ZNIRAIwClkam-8u@zte.com.cn?part=4

[PATCH 5/5] nvme: bound the reservation report retry loop

[shao.mingyin]

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

nvme_pr_read_reservation() first reads the reservation status header to
learn the number of registrants, allocates a buffer of that size, and
then reads the full status.  If the registrant count changed in between,
it frees the buffer and jumps back to get_num_regs.  There is no limit on
the number of retries:

	if (num_regs != get_unaligned_le16(&rse->regctl)) {
		kfree(rse);
		goto get_num_regs;
	}

The count comes from the controller and a target that keeps flipping it
(possible for a malicious NVMe-oF target) makes the ioctl loop forever.
Each iteration issues two Reservation Report commands and allocates up
to struct_size(..., 65535) bytes, and the calling process ends up in an
uninterruptible loop that cannot be killed.  Daemons that issue this
ioctl, such as cluster lock managers, get stuck with it.

Allow the retry only a few times and report -EAGAIN when the controller
still does not provide a consistent report.

Fixes: 28c97ba38ff9 ("nvme: Add pr_ops read_reservation 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
a TEST-ONLY nvmet hook that alternates the registrant count between the
header-only and the full Reservation Report.  The hook is compiled into
both kernels and only the host fix differs; no extra hardware is needed.

One PR key is registered, then the guest issues
IOC_PR_READ_RESERVATION.  The header report always says one registrant
and the full report says two, so the retry loop never converges:

  unpatched                            patched
  ioctl never returns (the runner      ioctl returns -EAGAIN after
  hits its timeout)                    three inconsistent report attempts

 drivers/nvme/host/pr.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
index fe7dbe264815..fab3bad2e2e0 100644
--- a/drivers/nvme/host/pr.c
+++ b/drivers/nvme/host/pr.c
@@ -275,7 +275,7 @@ static int nvme_pr_read_reservation(struct block_device *bdev,
 		struct pr_held_reservation *resv)
 {
 	struct nvme_reservation_status_ext tmp_rse, *rse;
-	int ret, i, num_regs;
+	int ret, i, num_regs, retries = 0;
 	u32 rse_len;
 	bool eds;
 
@@ -305,6 +305,8 @@ static int nvme_pr_read_reservation(struct block_device *bdev,
 
 	if (num_regs != get_unaligned_le16(&rse->regctl)) {
 		kfree(rse);
+		if (++retries >= 3)
+			return -EAGAIN;
 		goto get_num_regs;
 	}
 
-- 
2.43.0