[PATCH v2 0/2] nvmet: fix file backend short I/O and LBA range checks

[PATCH v2 0/2] nvmet: fix file backend short I/O and LBA range checks

[shao.mingyin]

The file backend computes byte offsets from the LBA and block size with
unchecked 64-bit arithmetic and does not handle short reads or writes.
A host can therefore make the backend report success for a transfer that
only covered part of the range, and a large LBA or length can wrap the
range check and reach vfs_fallocate() with an out-of-range offset.

Patch 1 makes nvmet_file_execute_io() stop at an incomplete submit
instead of advancing the position and eventually reporting success, and
converts a zero-byte completion on the common path to -EIO.  Patch 2
adds a shared overflow-safe range check and uses it for read, write,
discard and write zeroes.

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

Changes in v2 (Christoph Hellwig):
- Patch 2: use the shared LBA range helper for reads and writes, with
  pos64 local to the helper. No functional change from v1.
- Patch 1: unchanged; add Christoph Hellwig's Reviewed-by tag.
Link to v1: https://lore.kernel.org/all/20261111224134934XG6vMtntrNJQL1ztj7imr@zte.com.cn/

Shao Mingyin (2):
  nvmet: handle short reads and writes in the file backend
  nvmet: use overflow-safe range checks in the file backend

 drivers/nvme/target/io-cmd-file.c | 47 ++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 13 deletions(-)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

[PATCH v2 1/2] nvmet: handle short reads and writes in the file backend

[shao.mingyin]

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

When the file backend falls back to its bvec mempool, large requests
are submitted in synchronous chunks.  A short completion advances the
file position by the full chunk length and is eventually overwritten
with the total request length.  The target reports success for an
incomplete write or returns unread buffer contents as valid data.

The common completion path has another short-I/O case: a zero-byte
return, such as a read at EOF after the backing file is truncated, is
passed to errno_to_nvme_status() and becomes success.

Stop at any incomplete chunk and convert nonnegative short completions
to -EIO in both paths.  Preserve negative errors and full completions.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Fixes: c6aa3542e010 ("nvmet: add error log support for file backend")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Changes in v2:
- Code unchanged; add the Christoph Hellwig Reviewed-by tag.
Previous version: https://lore.kernel.org/all/20261111224245786kBVGPR0wSFFqHjmtHwtAJ@zte.com.cn/

Tested on 7.3.0-next-20261030 (QEMU/TCG) with the loop transport and a
file-backed namespace on tmpfs (buffered_io=1).

The chunked path is only taken when the bio_vec allocation falls back
to the mempool, so failslab was used to fail that allocation, limited to
nvmet_file_execute_rw() with the stack-trace filter (times=1,
ignore-gfp-wait=N).  The short completion was made deterministic by
shrinking the backing file behind nvmet's back (ns->size is only
revalidated on enable) and by leaving only 4 KiB of space in the tmpfs
for the last 64 KiB chunk of a 128 KiB write.  The values below are the
status words returned by NVME_IOCTL_IO_CMD:

                              unpatched    patched
  read  512 KiB               ret=0        ret=24582 (0x6006)
  read  256 KiB               ret=0        ret=24582
  read  128 KiB               ret=0        ret=24582
  write 128 KiB               ret=0        ret=24582

0x6006 contains NVME_SC_INTERNAL plus the Do Not Retry and More bits;
the More bit records the associated error-log entry.
Without the patch the reads report success although the tail of the
host buffer still holds the 0x5a pattern, and the write reports success
although only 68 KiB of 128 KiB reached the file.  Controls: without the
injection the short read already fails with 0x6006 on both kernels
(non-chunked path), and with the injection but a full-length read both
return 0.

 drivers/nvme/target/io-cmd-file.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 695963dd932ee758b7b89cf7a3d017407a3a6716..e95ab1f08cff39b0a1147e53bbd7749a831d29a6 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -113,7 +113,7 @@ static void nvmet_file_io_done(struct kiocb *iocb, long ret)
 	}
 
 	if (unlikely(ret != req->transfer_len))
-		status = errno_to_nvme_status(req, ret);
+		status = errno_to_nvme_status(req, ret < 0 ? ret : -EIO);
 	nvmet_req_complete(req, status);
 }
 
@@ -150,8 +150,11 @@ static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 		if (unlikely(is_sync) &&
 		    (nr_bvec - 1 == 0 || bv_cnt == NVMET_MAX_MPOOL_BVEC)) {
 			ret = nvmet_file_submit_bvec(req, pos, bv_cnt, len, 0);
-			if (ret < 0)
+			if (ret != len) {
+				if (ret >= 0)
+					ret = -EIO;
 				goto complete;
+			}
 
 			pos += len;
 			bv_cnt = 0;

[PATCH v2 2/2] nvmet: use overflow-safe range checks in the file backend

[shao.mingyin]

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

The file backend shifts a host-supplied SLBA to form a byte offset
before checking it against the namespace size. The shift can discard
high bits: with a 4 KiB block size, SLBA 2^52 + 1 becomes offset 4096.
The range check then accepts a command aimed outside the namespace and
the backend accesses data at the wrapped offset.

Reject overflowing LBA-to-byte conversions and check the transfer
length against the remaining namespace size without adding it to the
offset. Apply the checks to reads, writes, discard and write zeroes.

Fixes: 9c891c139894 ("nvmet: check fileio lba range access boundaries")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Changes in v2 (Christoph Hellwig):
- Share nvmet_file_check_lba_range() with read/write and hide pos64
  inside it. Pass byte lengths; check block-to-byte length conversion
  separately for discard and write zeroes. No functional change.
v1: https://lore.kernel.org/all/20261111224247129g0VbfWdGlwDMhBpChnT-8@zte.com.cn/

A userspace test compiled the range helper and the kernel shift-overflow
macro with GCC 13 and UBSAN. Boundary and random inputs matched the v1
checks and a 128-bit arithmetic oracle, including wrapped LBAs, byte and
block lengths, exact-end access and one-byte overruns.

 drivers/nvme/target/io-cmd-file.c | 40 ++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index e95ab1f08cff39b0a1147e53bbd7749a831d29a6..3098b902f51987f3da30dcc3cbb47729f62f7718 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -117,6 +117,19 @@ static void nvmet_file_io_done(struct kiocb *iocb, long ret)
 	nvmet_req_complete(req, status);
 }
 
+static bool nvmet_file_check_lba_range(struct nvmet_req *req, u64 slba,
+				       u64 len, loff_t *pos)
+{
+	u64 pos64;
+
+	if (check_shl_overflow(slba, req->ns->blksize_shift, &pos64) ||
+	    pos64 > req->ns->size || len > req->ns->size - pos64)
+		return false;
+
+	*pos = pos64;
+	return true;
+}
+
 static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 {
 	ssize_t nr_bvec = req->sg_cnt;
@@ -131,8 +144,9 @@ static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 	if (req->f.mpool_alloc && nr_bvec > NVMET_MAX_MPOOL_BVEC)
 		is_sync = true;
 
-	pos = le64_to_cpu(req->cmd->rw.slba) << req->ns->blksize_shift;
-	if (unlikely(pos + req->transfer_len > req->ns->size)) {
+	if (unlikely(!nvmet_file_check_lba_range(req,
+						 le64_to_cpu(req->cmd->rw.slba),
+						 req->transfer_len, &pos))) {
 		ret = -ENOSPC;
 		goto complete;
 	}
@@ -282,19 +296,22 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
 	int i;
 
 	for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
+		u64 len64;
+
 		status = nvmet_copy_from_sgl(req, i * sizeof(range), &range,
 					sizeof(range));
 		if (status)
 			break;
 
-		offset = le64_to_cpu(range.slba) << req->ns->blksize_shift;
-		len = le32_to_cpu(range.nlb);
-		len <<= req->ns->blksize_shift;
-		if (offset + len > req->ns->size) {
+		if (check_shl_overflow((u64)le32_to_cpu(range.nlb),
+				       req->ns->blksize_shift, &len64) ||
+		    !nvmet_file_check_lba_range(req, le64_to_cpu(range.slba),
+					len64, &offset)) {
 			req->error_slba = le64_to_cpu(range.slba);
 			status = errno_to_nvme_status(req, -ENOSPC);
 			break;
 		}
+		len = len64;
 
 		ret = vfs_fallocate(req->ns->file, mode, offset, len);
 		if (ret && ret != -EOPNOTSUPP) {
@@ -339,16 +356,17 @@ static void nvmet_file_write_zeroes_work(struct work_struct *w)
 	int mode = FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE;
 	loff_t offset;
 	loff_t len;
+	u64 len64;
 	int ret;
 
-	offset = le64_to_cpu(write_zeroes->slba) << req->ns->blksize_shift;
-	len = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
-			req->ns->blksize_shift);
-
-	if (unlikely(offset + len > req->ns->size)) {
+	if (check_shl_overflow((u64)le16_to_cpu(write_zeroes->length) + 1,
+			       req->ns->blksize_shift, &len64) ||
+	    !nvmet_file_check_lba_range(req, le64_to_cpu(write_zeroes->slba),
+				len64, &offset)) {
 		nvmet_req_complete(req, errno_to_nvme_status(req, -ENOSPC));
 		return;
 	}
+	len = len64;
 
 	ret = vfs_fallocate(req->ns->file, mode, offset, len);
 	nvmet_req_complete(req, ret < 0 ? errno_to_nvme_status(req, ret) : 0);

Re: [PATCH v2 1/2] nvmet: handle short reads and writes in the file backend

[Christoph Hellwig]

Still looks good:

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

Re: [PATCH v2 2/2] nvmet: use overflow-safe range checks in the file backend

[Christoph Hellwig]

Looks good:

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