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

[Niklas Cassel <cassel@kernel.org>]

On Fri, Sep 18, 2026 at 08:33:07AM +0100, John Garry wrote:
> 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?

Yes, hence the Assisted-by tag just a few lines further down:

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

The commit message does point out two actual problems resulting from the
missing check_device_access_params() call in resp_atomic_write() (which
exists in all other resp_write_*() functions):

- Fails to repect the write protect module parameter, so resp_atomic_write()
  fails to generate the proper sense data in this case.

- Fails to check if the command will write past that device capacity, so
  resp_atomic_write() fails to generate the proper sense data in this case.

The generated sense data differs in the two cases.

I suppose we could drop:

  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.


From the commit message, as I suppose it might be a bit overly verbose to
mention the exact sense data that each missing check would generate.
If someone cares about that, they can just look at the mk_sense_buffer()
calls in check_device_access_params().


Kind regards,
Niklas