[PATCH RESEND v4 1/4] ata: libata-scsi: reject reserved bits in 32-byte passthru
From: shao.mingyin
Date: Fri Nov 13 2026 - 16:01:34 EST
From: Shao Mingyin <shao.mingyin@xxxxxxxxxx>
ATA PASS-THROUGH(32) has no MULTIPLE_COUNT field. Byte 1 is CONTROL,
including vendor-specific bits 7:6, but libata interprets bits 7:5 as
MULTIPLE_COUNT. This rejects non-multiple commands with those bits set
and can print spurious count mismatch warnings for multiple commands.
The reserved bits 7:5 of byte 10 are not checked.
Reject nonzero reserved bits in byte 10 with field pointer 10. Skip
the MULTIPLE_COUNT checks for the 32-byte form while retaining the
device multiple-mode check for READ/WRITE MULTIPLE commands. Also point
to byte 10 for invalid PROTOCOL. Keep the 12- and 16-byte count checks
unchanged.
Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
Suggested-by: Niklas Cassel <cassel@xxxxxxxxxx>
Signed-off-by: Shao Mingyin <shao.mingyin@xxxxxxxxxx>
---
Changes in v4 (Niklas Cassel and Damien Le Moal):
- Check byte 10 bits 7:5 and report fp=10. CONTROL byte 1 is not a
MULTIPLE_COUNT field and its bits 7:6 are vendor specific.
- Skip the short-CDB MULTIPLE_COUNT check for ATA-32, retain support
for READ/WRITE MULTIPLE, and correct the misleading comment.
- Split ata_dev_warn() after dev, as requested by Damien.
- Do not carry the v3 Reviewed-by tag across this behavioral change.
Changes in v3 (Damien Le Moal):
- Added a byte-1 bits 7:5 check with fp=1. This checked CONTROL
instead of the reserved bits and is corrected in v4.
- Retained the short-CDB count and device multiple-mode checks.
- Updated the subject, description and CONTROL-byte test expectations.
Changes in v2 (Niklas Cassel):
- Skip the nonexistent MULTIPLE_COUNT field for the 32-byte CDB;
preserve the device multiple-mode check and the short-CDB checks.
- Move the invalid-PROTOCOL field pointer fix here from patch 3.
- Regenerate with git format-patch; do not edit diff hunks or indexes.
- Do not carry the v1 Reviewed-by tag over the reworked code.
v3: https://lore.kernel.org/all/20261112220120113cXLhRk6iDVvhfgGPoZ5vJ@xxxxxxxxxx/
v2: https://lore.kernel.org/all/20261111223129430zClEJF8TDgu_h8ZYM843Q@xxxxxxxxxx/
v1: https://lore.kernel.org/all/20261104220939917jqg8_alGN5sACGWq5M9D-@xxxxxxxxxx/
Review: https://lore.kernel.org/all/avcujpOaXJPfjgxQ@ryzen/
Validation on x86-64 with GCC 13 and UBSAN: a userspace harness compiles
the translation and completion functions verbatim with SCSI/device
stubs. It crosses all 256 CONTROL values and all eight byte-10 high-bit
patterns for non-multiple and READ/WRITE MULTIPLE commands, checking
rejection with field pointer 10 and no CONTROL-based count warnings.
It also checks the device multiple-mode and 12-/16-byte count checks,
service actions, CK_COND and field pointers. Sense construction and
device access use stubs; this is not a kernel runtime test.
drivers/ata/libata-scsi.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb03312793ca94d81d8061a16c66e786c4cf3..8d321197803a839d9586e9f1436d1fbe3f5383c6 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3388,7 +3388,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]);
if (tf->protocol == ATA_PROT_UNKNOWN) {
- fp = 1;
+ fp = 1 + cdb_offset;
goto invalid_fld;
}
@@ -3573,27 +3573,34 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
goto invalid_fld;
}
- /* sanity check for pio multi commands */
- if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
+ /* Sanity check for pio multi commands. */
+ if (cdb[0] == VARIABLE_LENGTH_CMD && (cdb[10] & 0xe0)) {
+ fp = 10;
+ goto invalid_fld;
+ }
+
+ if (cdb[0] != VARIABLE_LENGTH_CMD &&
+ (cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
fp = 1;
goto invalid_fld;
}
if (is_multi_taskfile(tf)) {
- unsigned int multi_count = 1 << (cdb[1] >> 5);
-
if (!dev->multi_count) {
ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND,
NO_ADDITIONAL_SENSE_INFORMATION);
return 1;
}
- /* compare the passed through multi_count
- * with the cached multi_count of libata
- */
- if (multi_count != dev->multi_count)
- ata_dev_warn(dev, "invalid multi_count %u ignored\n",
- multi_count);
+ if (cdb[0] != VARIABLE_LENGTH_CMD) {
+ unsigned int multi_count = 1 << (cdb[1] >> 5);
+
+ /* Compare with the cached multiple count of libata. */
+ if (multi_count != dev->multi_count)
+ ata_dev_warn(dev,
+ "invalid multi_count %u ignored\n",
+ multi_count);
+ }
}
/*
--
2.27.0