Tejun Heo wrote:
libata depended on SCSI command to have the correct length when
tranlating it into an ATA command. This generally worked for commands
issued by SCSI HLD but user could issue arbitrary broken command using
sg interface.
Also, when building ATAPI command, full command size was always
copied. Because some ATAPI devices needs bytes after CDB cleared, if
upper layer doesn't clear bytes after CDB, such devices will
malfunction. This necessiated recent clear-garbage-after-CDB fix in
sg interfaces. However, scsi_execute() isn't fixed yet and HL-DT-ST
DVD-RAM GSA-H30N malfunctions on initialization commands issued from
SCSI.
This patch updates libata xlat functions.
* SCSI cmd_len is always considered. Each translation function checks
for proper cmd_len and ATAPI translaation clears bytes after CDB.
* Don't pass CDB as argument to xlat functions. xlat functions need
to access more than just CDB and they have already been accessing scmd
via qc->scsicmd. Just pass in qc.
* Variable names are normalized. scsi_cmnd is scmd and CDB, cdb.
Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
Cc: Jens Axboe <jens.axboe@xxxxxxxxxx>
Cc: Douglas Gilbert <dougg@xxxxxxxxxx>
---
drivers/ata/libata-scsi.c | 187 ++++++++++++++++++++++++---------------------
1 files changed, 100 insertions(+), 87 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a4790be..f82871c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -51,7 +51,7 @@
#define SECTOR_SIZE 512
-typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc, const u8 *scsicmd);
+typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc);
static struct ata_device * __ata_scsi_find_dev(struct ata_port *ap,
const struct scsi_device *scsidev);
@@ -935,7 +935,6 @@ int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth)
/**
* ata_scsi_start_stop_xlat - Translate SCSI START STOP UNIT command
* @qc: Storage for translated ATA taskfile
- * @scsicmd: SCSI command to translate
*
* Sets up an ATA taskfile to issue STANDBY (to stop) or READ VERIFY
* (to start). Perhaps these commands should be preceded by
@@ -948,22 +947,25 @@ int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth)
* RETURNS:
* Zero on success, non-zero on error.
*/
-
-static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc,
- const u8 *scsicmd)
+static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
{
+ struct scsi_cmnd *scmd = qc->scsicmd;
+ const u8 *cdb = scmd->cmnd;
struct ata_taskfile *tf = &qc->tf;
+ if (scmd->cmd_len < 5)
+ goto invalid_fld;
+
tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
tf->protocol = ATA_PROT_NODATA;
- if (scsicmd[1] & 0x1) {
+ if (cdb[1] & 0x1) {
; /* ignore IMMED bit, violates sat-r05 */
}
- if (scsicmd[4] & 0x2)
- goto invalid_fld; /* LOEJ bit set not supported */
- if (((scsicmd[4] >> 4) & 0xf) != 0)
- goto invalid_fld; /* power conditions not supported */
- if (scsicmd[4] & 0x1) {
+ if (cdb[4] & 0x2)
+ goto invalid_fld; /* LOEJ bit set not supported */
+ if (((cdb[4] >> 4) & 0xf) != 0)
+ goto invalid_fld; /* power conditions not supported */
+ if (cdb[4] & 0x1) {
ACK change for #upstream-fixes, but please revise so that the
"s/scsicmd/cdb/" rename is moved to a separate patch. You should be
able to name the local variables such that this patch becomes much smaller.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
