Re: [PATCH] ata: libata-scsi: reject multi-sector taskfile commands when dev->multi_count is 0

[Damien Le Moal]

On 9/20/26 07:26, Hui Peng wrote:
> In ata_scsi_pass_thru() (drivers/ata/libata-scsi.c), issuing
> ATA_CMD_READ_MULTI / ATA_CMD_WRITE_MULTI / ATA_CMD_READ_MULTI_EXT /
> ATA_CMD_WRITE_MULTI_EXT / ATA_CMD_WRITE_MULTI_FUA_EXT via SG_IO when
> dev->multi_count == 0 sets qc->sect_size = 0 and triggers a divide-by-

Where do you see that?

> zero or WARN_ON in the ATA PIO/taskfile path. Reject ATA_PROT_PIO multi-
> sector commands with -EINVAL when dev->multi_count == 0.

That is not what your patch is doing.

> 
> Fixes: c6fd280766a0 ("Move libata to drivers/ata.")
> Assisted-by: LLM

See below, but you should tell your AI to read the ACS and SAT specs first. Or
better: check the specs *yourself* and analyze what is said before sending
patches that are not correct.

> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>
> ---
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index b3666519b648..c99f2845d09b 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3507,6 +3507,11 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
>  	if (is_multi_taskfile(tf)) {
>  		unsigned int multi_count = 1 << (cdb[1] >> 5);
>  
> +		if (!dev->multi_count) {
> +			fp = (cdb[0] == ATA_16) ? 14 : 9;

ACS-3, which is the last ACS specification version without an obsolete
definition of multi-sector PIO read/wrtite states that:

If IDENTIFY DEVICE data word 59 bit 8 (see 7.12.7.21) is cleared to zero, and a
READ MULTIPLE EXT command is received by the device, and the device has not
returned command completion without an error for a SET MULTIPLE MODE command,
the device shall return command aborted.

The same statement is also present for WRITE MULTIPLE command.

Now referring to the SAT specifications, you can see that an ABRT (aborted
command) error should be reported as failed with the ABORTED COMMAND sense key
and NO ADDITIONAL SENSE INFORMATION additional sense code.
So here, the correct error to return is not invalid field.

> +			goto invalid_fld;
> +		}
> +
>  		/* compare the passed through multi_count
>  		 * with the cached multi_count of libata
>  		 */


-- 
Damien Le Moal
Western Digital Research