Re: [PATCH 1/3] ata: libata-scsi: fix the multi_count check for 32-byte passthru

[Damien Le Moal <dlemoal@kernel.org>]

On 11/4/26 23:09, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> ata_scsi_pass_thru() translates 12, 16 and 32-byte ATA pass-through CDBs.
> For a 32-byte CDB (VARIABLE_LENGTH_CMD with the ATA_32 service action) the
> ATA command descriptor starts at byte offset 9, so the function adds
> cdb_offset when reading the protocol (cdb[1 + cdb_offset]) and T_LENGTH
> (cdb[2 + cdb_offset]) fields.
> 
> The PIO multi-sector sanity check and the multi_count computation,
> however, still read cdb[1] without adding cdb_offset:
> 
> 	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf))
> 	...
> 	unsigned int multi_count = 1 << (cdb[1] >> 5);
> 
> For a 32-byte CDB, cdb[1] is the variable length CDB control byte and
> its bits 7:5 do not carry MULTIPLE_COUNT.  As a result, the sanity check
> that rejects a multi-sector count on a non-multi taskfile is silently
> skipped, and multi_count is always computed as 1 << 0 = 1, so the
> comparison against dev->multi_count either emits a spurious
> "invalid multi_count 1 ignored" warning for devices configured with a
> different multi-sector size or misses a real mismatch.
> 
> Read the count field from cdb[1 + cdb_offset], matching the rest of the
> function, and report the field pointer relative to the same byte so that
> a rejected multi-sector count points at the MULTIPLE_COUNT field instead
> of byte 1.  For 12 and 16-byte CDBs cdb_offset is zero, so their
> behavior is unchanged.
> 
> Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research