From: Shao Mingyin Several error paths in ata_scsi_pass_thru() report field pointers for the 12- or 16-byte CDB when rejecting a 32-byte CDB. Initiators then receive INVALID FIELD IN CDB sense data pointing at an unrelated byte. READ/WRITE LONG also reports PROTOCOL when its sector count is invalid. Use the PROTOCOL offset and record the FEATURES, SECTOR COUNT and COMMAND offsets for each CDB format. Report SECTOR COUNT separately from PROTOCOL when validating READ/WRITE LONG. Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.") Suggested-by: Niklas Cassel Reviewed-by: Damien Le Moal Signed-off-by: Shao Mingyin --- Changes in v3: - Code unchanged; retain the Damien Le Moal Reviewed-by tag. Previous version: https://lore.kernel.org/all/20261111223133188VjHq0FbmnzuEZWakS-EYD@zte.com.cn/ Changes in v2 (Niklas Cassel): - Add a separate patch for the remaining PROTOCOL, FEATURES, COMMAND and READ/WRITE LONG sector-count field pointers. v1: https://lore.kernel.org/all/20261104220832716LXKf7OaS9dt1VJ8Q959AG@zte.com.cn/ Validation on x86-64 with GCC 13 and UBSAN: a userspace harness compiled the translation functions verbatim with SCSI/device stubs. The rejected 32-byte DMA/NCQ and READ/WRITE LONG protocols passed field pointer 10 to the invalid-field helper, SET FEATURES passed 21, and TPM passed 25. Invalid READ/WRITE LONG sector counts passed 4, 6 and 23 for 12-, 16- and 32-byte CDBs. These are helper arguments observed in the harness. drivers/ata/libata-scsi.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 8ee68937e884369beb4d7ad218612d28c5a080ff..c59e6567a108bbb353c08e921228eadbd231da63 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3383,6 +3383,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) struct ata_device *dev = qc->dev; const u8 *cdb = scmd->cmnd; u16 fp; + u8 feature_offset, nsect_offset, command_offset; u16 cdb_offset = 0; /* 7Fh variable length cmd means a ata pass-thru(32) */ @@ -3443,6 +3444,9 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) tf->lbah = cdb[12]; tf->device = cdb[13]; tf->command = cdb[14]; + feature_offset = 4; + nsect_offset = 6; + command_offset = 14; break; case ATA_12: /* @@ -3457,6 +3461,9 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) tf->lbah = cdb[7]; tf->device = cdb[8]; tf->command = cdb[9]; + feature_offset = 3; + nsect_offset = 4; + command_offset = 9; break; default: /* @@ -3482,6 +3489,9 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) tf->device = cdb[24]; tf->command = cdb[25]; tf->auxiliary = get_unaligned_be32(&cdb[28]); + feature_offset = 21; + nsect_offset = 23; + command_offset = 25; break; } @@ -3499,8 +3509,12 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) case ATA_CMD_READ_LONG_ONCE: case ATA_CMD_WRITE_LONG: case ATA_CMD_WRITE_LONG_ONCE: - if (tf->protocol != ATA_PROT_PIO || tf->nsect != 1) { - fp = 1; + if (tf->protocol != ATA_PROT_PIO) { + fp = 1 + cdb_offset; + goto invalid_fld; + } + if (tf->nsect != 1) { + fp = nsect_offset; goto invalid_fld; } qc->sect_size = scsi_bufflen(scmd); @@ -3566,13 +3580,13 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) /* We may not issue DMA commands if no DMA mode is set */ if (tf->protocol == ATA_PROT_DMA && !ata_dma_enabled(dev)) { - fp = 1; + fp = 1 + cdb_offset; goto invalid_fld; } /* We may not issue NCQ commands to devices not supporting NCQ */ if (ata_is_ncq(tf->protocol) && !ata_ncq_enabled(dev)) { - fp = 1; + fp = 1 + cdb_offset; goto invalid_fld; } @@ -3613,7 +3627,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) */ if (tf->command == ATA_CMD_SET_FEATURES && tf->feature == SETFEATURES_XFER) { - fp = (cdb[0] == ATA_16) ? 4 : 3; + fp = feature_offset; goto invalid_fld; } @@ -3633,7 +3647,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc) * can turn off TC features of their system. */ if (tf->command >= 0x5C && tf->command <= 0x5F && !libata_allow_tpm) { - fp = (cdb[0] == ATA_16) ? 14 : 9; + fp = command_offset; goto invalid_fld; }