Re: [PATCH v6 05/11] scsi: scsi_debug: Report the residual of a write

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi_debug: residual reporting bypassed in resp_write_scat() edge cases
--

commit 2c4fb6bce2173fbd341e31abb8babf6bd210996d
Author: Niklas Cassel <cassel@kernel.org>

scsi: scsi_debug: Report the residual of a write

A write whose data buffer is larger than its transfer length leaves a
residual that the initiator is entitled to be told about. resp_read_dt0()
and resp_write_tape() report it, but the write paths for a disk do not,
so such a write completes with GOOD status and a residual of zero while
a READ of the same length into the same buffer reports it correctly.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index b64ae3ad300d8..fd80f66a86ac9 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -5350,6 +5352,13 @@ static int resp_write_scat(struct scsi_cmnd *scp,
>  		sg_off += num_by;
>  		cum_lb += num;
>  	}
> +	/*
> +	 * sg_off counts what the command asked for, which can exceed the
> +	 * buffer: lbdof is not validated against it, and a range is counted
> +	 * in full even if do_device_access() copied less.
> +	 */
> +	if (scsi_bufflen(scp) > sg_off)
> +		scsi_set_resid(scp, scsi_bufflen(scp) - sg_off);
>  	ret = 0;
>  err_out_unlock:
>  	sdeb_meta_write_unlock(sip);

[Severity: Medium]
Will this miss reporting residuals when resp_write_scat() returns early or
takes an error path?

For instance, if the initiator provides a data-out buffer but the command
has 0-length requests, the early return path in resp_write_scat() bypasses
the new residual reporting logic entirely:

	if ((num_lrd == 0) || (bt_len == 0))
		return 0;       /* T10 says these do-nothings are not errors */

Similarly, if error injection for recovered errors triggers during the
descriptor processing loop, the code jumps directly to err_out_unlock,
which bypasses the new residual calculation (unlike resp_write_dt0() where
the residual is set before the error injection block):

			if (sdebug_opts & SDEBUG_OPT_RECOVERED_ERR) {
				mk_sense_buffer(scp, RECOVERED_ERROR,
					FAILURE_PREDICTION_THRESHOLD_EXCEEDED);
				atomic_set(&sdeb_inject_pending, 0);
				ret = check_condition_result;
				goto err_out_unlock;

Could this cause an incorrect residual of 0 to be reported in these edge
cases, indicating to the initiator that the entire data-out buffer was
consumed when it was not?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924112127.3815255-13-cassel@kernel.org?part=5