Re: [PATCH v2] ata: libata-scsi: do not lose CHECK CONDITION for failed ATAPI commands

[Damien Le Moal]

On 2026/09/29 18:41, hengyul@xxxxxxxxxx wrote:
> From: Hengyu Liang <hengyul@xxxxxxxxxx>
> 
> Since commit 2e1d2e65e773 ("ata: libata-scsi: terminate deferred commands
> on time out"), atapi_qc_complete() sets SAM_STAT_CHECK_CONDITION for a
> failed command only if cmd->result is zero, so that the DID_TIME_OUT or
> DID_REQUEUE host byte of a terminated command is preserved.
> 
> However, cmd->result can also be non-zero for a regular failed ATAPI
> command completed through libata EH: ata_eh_analyze_tf() calls
> ata_eh_decide_disposition(), that is, scsi_check_sense(), on the sense
> data obtained with REQUEST SENSE, and scsi_check_sense() sets the SCSI
> midlayer internal byte of the result for some sense codes, e.g.
> SCSIML_STAT_TGT_FAILURE for ILLEGAL REQUEST with ASC 0x20 (INVALID
> COMMAND OPERATION CODE) or 0x24 (INVALID FIELD IN CDB), or
> SCSIML_STAT_MED_ERROR for MEDIUM ERROR with ASC 0x11 (UNRECOVERED READ
> ERROR). For such commands, atapi_qc_complete() keeps the result as is and
> the command completes with a GOOD status byte, even though it failed and
> valid sense data is available.
> 
> As a result, passthrough users see these failed ATAPI commands as
> successful. For example, with the emulated IDE CD-ROM drive of QEMU and
> no medium loaded, a PLAY AUDIO MSF command issued with SG_IO completes
> with status 0x00 (and no DRIVER_SENSE) instead of 0x02 (CHECK CONDITION)
> with sense key ILLEGAL REQUEST, ASC/ASCQ 0x20/0x00, and the CDROM ioctls
> CDROMPLAYTRKIND, CDROMSUBCHNL, CDROMPAUSE, CDROMRESUME, CDROMPLAYMSF and
> CDROM_GET_MCN return 0 instead of -ENOMEDIUM or -EOPNOTSUPP.
> 
> Fix this by setting the status byte to SAM_STAT_CHECK_CONDITION whenever
> the host byte is not set. Use set_status_byte() rather than overwriting
> the whole result, so that the SCSI midlayer internal byte set by
> scsi_check_sense() is preserved, as ata_scsi_qc_complete() already does
> for ATA commands.
> 
> Fixes: 2e1d2e65e773 ("ata: libata-scsi: terminate deferred commands on time out")
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Niklas Cassel <cassel@xxxxxxxxxx>
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>

Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>

-- 
Damien Le Moal
Western Digital Research