Re: [PATCH v5 30/40] scsi: ibmvscsi_tgt: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] When `srp_transfer_data()` fails during an RDMA read data transfer to the initiator, the target populates the sense buffer but fails to update the command status to `SAM_STAT_CHECK_CONDITION`. This causes the target to send a `SAM_STAT_GOOD` response to the initiator, masking the transfer failure and causing the initiator to process stale/corrupted data from its unmodified read buffers.
--

commit 6317be85b00b8041ec78e59ae2da48d7e5d24fde
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: ibmvscsi_tgt: use 16-bits defined sense codes

Refactor the IBM virtual SCSI target driver to use scsi_set_sense_buffer()
and replace all hard-coded additional sense codes and additional sense
code qualifiers with the enum values defined in include/scsi/scsi_sense.h.
This helps with code clarity as the sense codes being processed are easier
to test and self-documented.

> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index 61f6828007654..e2af90b6da39b 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -3757,9 +3757,8 @@ static int ibmvscsis_queue_data_in(struct se_cmd *se_cmd)
>  		dev_err(&vscsi->dev, "srp_transfer_data failed: %d\n", rc);
>  		se_cmd->scsi_sense_length = 18;
>  		memset(se_cmd->sense_buffer, 0, se_cmd->scsi_sense_length);
> -		/* Logical Unit Communication Time-out asc/ascq = 0x0801 */
> -		scsi_build_sense_buffer(0, se_cmd->sense_buffer, MEDIUM_ERROR,
> -					0x08, 0x01);
> +		scsi_set_sense_buffer(0, se_cmd->sense_buffer, MEDIUM_ERROR,
> +				      LU_COMMUNICATION_TIMEOUT);

[Severity: Critical]
This is a pre-existing issue, but when srp_transfer_data() fails during an
RDMA read data transfer, the code populates the sense buffer but fails to
update the command status to SAM_STAT_CHECK_CONDITION.

Without setting se_cmd->scsi_status = SAM_STAT_CHECK_CONDITION here, the target
core seems to believe the command succeeded.

Later in srp_build_response():

drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:srp_build_response() {
    ...
    if (cmd->type == SCSI_CDB) {
        rsp->status = ibmvscsis_fast_fail(vscsi, cmd);
        if (rsp->status) {
            ...
            if (se_cmd->scsi_sense_length && se_cmd->sense_buffer) {
                rsp->sense_data_len = cpu_to_be32(se_cmd->scsi_sense_length);
    ...
}

Because se_cmd->scsi_status is 0 (SAM_STAT_GOOD), rsp->status remains 0. The
check if (rsp->status) evaluates to false, causing the sense data to be
completely bypassed and ignored.

Could this cause the target to send a SAM_STAT_GOOD response to the initiator,
masking the transfer failure and causing the initiator to process stale or
corrupted data from its unmodified read buffers?

>  	}
>  
>  	srp_build_response(vscsi, cmd, &len);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907024343.1046387-1-dlemoal@kernel.org?part=30