Re: [PATCH 1/2] scsi: scsi_debug: handle protection information for WRITE ATOMIC(16)

[sashiko-bot@kernel.org]

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] scsi: scsi_debug: incorrect CDB byte offset in
  resp_atomic_write() [1]
--

commit 48252d9d72a1d4d7e89b4f529cfc1af6aee082f9
Author: Tal Zussman <tz2294@columbia.edu>

scsi: scsi_debug: handle protection information for WRITE ATOMIC(16)

This commit routes resp_atomic_write() through __resp_write_dt0() so that
protection information is verified and stored for WRITE ATOMIC(16)
commands.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index a922c8318a2fb..7d50650396377 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -6328,6 +6329,7 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>  	u16 boundary, len;
>  	u64 lba, lba_tmp;
>  	bool lbp = scsi_debug_lbp();
> +	int scsi_status = DID_ERROR << 16;
>  	int ret;
>  
>  	if (!scsi_debug_atomic_write()) {

[Severity: Medium]
This problem wasn't introduced by this patch, but does resp_atomic_write()
report the wrong CDB byte offset [1] when boundary exceeds
sdebug_atomic_wr_max_bndry?

drivers/scsi/scsi_debug.c:resp_atomic_write() {
	...
	boundary = get_unaligned_be16(cmd + 10);
	len = get_unaligned_be16(cmd + 12);
	...
	if (boundary > 0) {
		if (boundary > sdebug_atomic_wr_max_bndry) {
			mk_sense_invalid_fld(scp, SDEB_IN_CDB, 12, -1);
			return check_condition_result;
		}
	...
}

boundary is parsed from CDB byte 10, while byte 12 is the transfer length
field, so the sense data points the initiator at the transfer length
instead of the atomic boundary field.

[1] https://sashiko.dev/#/bug/linux-0276b62a-b78e-4b07-bc67-b1a9f88dc08e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-scsi-debug-pi-v1-0-539829902b3a@columbia.edu?part=1