Re: [PATCH v4 10/10] scsi: scsi_debug: Validate the access parameters of WRITE ATOMIC (16)

[John Garry <john.garry@linux.dev>]

On 9/18/26 07:29, Niklas Cassel wrote:
> resp_atomic_write() validates the fields that are specific to an atomic
> write, the alignment and granularity of the transfer, the atomic
> boundary and the maximum transfer length, but it never validates the
> range that the command addresses. Every other command that writes user
> data calls check_device_access_params() first, which rejects a transfer
> that ends beyond the capacity of the device, one whose length exceeds
> the size of the store, and any write to a write protected device.
> 
> As a consequence a WRITE ATOMIC (16) past the end of the device is not
> terminated with LOGICAL BLOCK ADDRESS OUT OF RANGE. do_device_access()
> reduces the LBA modulo the size of the store, so the command writes
> somewhere else on the medium instead. A WRITE ATOMIC (16) also writes to
> a device whose wp module parameter is set, which every other write
> refuses with DATA PROTECT.
> 
> Call check_device_access_params(). The zone checks that it ends with are
> unreachable, as atomic writes and ZBC emulation are mutually exclusive.

These are quite verbose commit messages ... LLM-generated, by chance?

> 
> Assisted-by: LLM
> Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: John Garry <john.garry@linux.dev>

> ---
> Tested with:
> 
>    modprobe scsi_debug sector_size=512 physblk_exp=3 dev_size_mb=128 \
>        atomic_wr=1
> 
> The device has a capacity of 262144 logical blocks. A WRITE ATOMIC (16)
> of eight blocks at LBA 262140 is now terminated with ILLEGAL REQUEST /
> LOGICAL BLOCK ADDRESS OUT OF RANGE; before this patch it completed with
> GOOD status, having written eight blocks at LBA 0 instead.
> 
> With the wp module parameter set to 1, a WRITE ATOMIC (16) is now
> terminated with DATA PROTECT / LOGICAL UNIT SOFTWARE WRITE PROTECTED,
> which is what an ordinary WRITE(16) has always returned; before this
> patch it completed with GOOD status and wrote the data.
> ---
>   drivers/scsi/scsi_debug.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 5243401701f9..5ae1241e4d8b 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6246,6 +6246,10 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>   		}
>   	}
>   
> +	ret = check_device_access_params(scp, lba, len, true);
> +	if (ret)
> +		return ret;
> +
>   	if (lbp) {
>   		sdeb_meta_write_lock(sip);
>   		meta_data_locked = true;