Re: [PATCH v1 4/4] scsi: sd: fix sd_done() sense handling condition

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a logic regression by unconditionally bypassing valid sense data processing for any SCSI status other than SAM_STAT_CHECK_CONDITION.
--

commit 6e9e6fa97257d1c3c528406c9719d2273c7f5a65
Author: Yang Xiuwei <yangxiuwei@kylinos.cn>

scsi: sd: fix sd_done() sense handling condition

This commit modifies the conditional in sd_done() to avoid entering the
sense_key switch with an uninitialized or stale sshdr when a CHECK CONDITION
occurs with invalid or deferred sense data. It replaces the previous logic
with an OR condition.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2419,8 +2419,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>  	}
>  	sdkp->medium_access_timed_out = 0;
>  
> -	if (!scsi_status_is_check_condition(result) &&
> -	    (!sense_valid || sense_deferred))
> +	if (!scsi_status_is_check_condition(result) ||
> +	    !sense_valid || sense_deferred)

[Severity: High]
Does this logic change inadvertently bypass valid sense data processing for
any SCSI status other than SAM_STAT_CHECK_CONDITION?

For example, if a low-level driver completes an I/O with
SAM_STAT_COMMAND_TERMINATED alongside valid sense data like RECOVERED_ERROR,
!scsi_status_is_check_condition(result) will evaluate to true.

This would cause the code to immediately jump to the 'out' label, skipping
the sense key switch block. Because good_bytes is initialized to 0 for any
non-zero result, the command will incorrectly fail instead of being handled
as fully or partially completed.

Would it be better to solely check if (!sense_valid || sense_deferred) to
ensure sshdr is initialized without discarding valid sense data on other
statuses?

>  		goto out;
>  
>  	switch (sshdr.sense_key) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623100159.4018066-1-yangxiuwei@kylinos.cn?part=4