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

[Christoph Hellwig <hch@lst.de>]

On Wed, Nov 11, 2026 at 10:42:47PM +0800, shao.mingyin@zte.com.cn wrote:
> 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>
> ---
> 
> Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the loop transport
> and a 4 MiB file-backed namespace.
> 
> A Write with SLBA = 2^52 + 1 and a 4 KiB block size yields a byte
> offset that wraps to 0x1000 and passes the size check:
> 
>   unpatched                            patched
>   ret=0 (write accepted at 0x1000)     ret=24705 (Capacity Exceeded)
>   drivers/nvme/target/io-cmd-file.c |   41 +++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index 695963dd932e..d59dbf61b1b0 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -125,17 +125,22 @@ static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
>  	size_t len = 0, total_len = 0;
>  	ssize_t ret = 0;
>  	loff_t pos;
> +	u64 pos64;
>  	int i;
>  	struct scatterlist *sg;
>  
>  	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(check_shl_overflow(le64_to_cpu(req->cmd->rw.slba),
> +					req->ns->blksize_shift,
> +					&pos64) ||
> +		     pos64 > req->ns->size ||
> +		     req->transfer_len > req->ns->size - pos64)) {

This condition looks like something that should be factored into a
helper to a bit more readable and so that the pos64 variable can be
hidden inside it.

Otherwise looks good.