mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ata: libata-scsi: reject multi-sector taskfile commands when dev->multi_count is 0
@ 2026-09-19 22:26 Hui Peng
  2026-09-20  4:32 ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Hui Peng @ 2026-09-19 22:26 UTC (permalink / raw)
  To: dlemoal, cassel; +Cc: linux-ide, linux-kernel

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

Fixes: c6fd280766a0 ("Move libata to drivers/ata.")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
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;
+			goto invalid_fld;
+		}
+
 		/* compare the passed through multi_count
 		 * with the cached multi_count of libata
 		 */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ata: libata-scsi: reject multi-sector taskfile commands when dev->multi_count is 0
  2026-09-19 22:26 [PATCH] ata: libata-scsi: reject multi-sector taskfile commands when dev->multi_count is 0 Hui Peng
@ 2026-09-20  4:32 ` Damien Le Moal
  2026-09-21  3:25   ` [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count " Hui Peng
  0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2026-09-20  4:32 UTC (permalink / raw)
  To: Hui Peng, cassel; +Cc: linux-ide, linux-kernel

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@gmail.com>
> ---
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count is 0
  2026-09-20  4:32 ` Damien Le Moal
@ 2026-09-21  3:25   ` Hui Peng
  2026-09-21  7:07     ` Damien Le Moal
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Hui Peng @ 2026-09-21  3:25 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel; +Cc: linux-ide, linux-kernel, stable, Hui Peng

When an ATA_12, ATA_16, or ATA_32 pass-through command with a
multi-sector PIO command opcode (ATA_CMD_READ_MULTI,
ATA_CMD_WRITE_MULTI, ATA_CMD_READ_MULTI_EXT, ATA_CMD_WRITE_MULTI_EXT, or
ATA_CMD_WRITE_MULTI_FUA_EXT) is submitted via SG_IO on a device where
multiple-sector mode is not configured (dev->multi_count == 0),
ata_scsi_pass_thru() currently logs a warning and still dispatches the
taskfile. When the DRQ interrupt fires, ata_pio_sectors() triggers
WARN_ON_ONCE(qc->dev->multi_count == 0) and computes
nsect = min((qc->nbytes - qc->curbytes) / qc->sect_size, 0) = 0, failing
to transfer any sectors:

  ata1.00: invalid multi_count 1 ignored
  WARNING: drivers/ata/libata-sff.c:666 at ata_pio_sectors+0x27d/0x300
  Call Trace:
   <IRQ>
   ata_sff_hsm_move+0x211/0x22e0
   __ata_sff_port_intr+0x1c8/0x520
   ata_bmdma_port_intr+0xa1/0x5b0
   ata_bmdma_interrupt+0x1f5/0x550

Per ACS-3 section 7.12.7.21, if IDENTIFY DEVICE word 59 bit 8 is cleared
to zero (multi_count == 0) and a READ MULTIPLE or WRITE MULTIPLE command
is received without a preceding successful SET MULTIPLE MODE command, the
device returns command aborted (ABRT). Per the SAT specification, an ABRT
error translates to the ABORTED COMMAND sense key with NO ADDITIONAL SENSE
INFORMATION (0x00, 0x00).

Fail multi-sector taskfile commands in ata_scsi_pass_thru() with
ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0) when
dev->multi_count == 0.

Tested in QEMU against Linux 7.3.0-rc3 (-device ide-cf, where
dev->multi_count is 0) by issuing ATA_CMD_SET_MULTI (nsect=1) followed
by an SG_IO ATA_16 ATA_CMD_READ_MULTI command: on the unfixed kernel
this triggers WARNING: drivers/ata/libata-sff.c:666 in
ata_pio_sectors(), whereas on the fixed kernel it immediately completes
with sense key ABORTED_COMMAND (0x0b, 0x00, 0x00) and 0 warnings.

Fixes: 1dce589c38c3 ("libata passthru: support PIO multi commands")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Report sense key ABORTED_COMMAND (0x00, 0x00) via ata_scsi_set_sense()
  per ACS-3 and SAT specifications instead of ILLEGAL_REQUEST /
  invalid_fld, as pointed out by Damien Le Moal.
- Update the commit description to accurately describe the
  ata_pio_sectors() WARN_ON_ONCE(qc->dev->multi_count == 0) and 0-sector
  transfer path and fix the Fixes: tag to 1dce589c38c3.

 drivers/ata/libata-scsi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b3666519b648..29fd6d8d97ce 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) {
+			ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0);
+			return 1;
+		}
+
 		/* compare the passed through multi_count
 		 * with the cached multi_count of libata
 		 */
-- 
2.49.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count is 0
  2026-09-21  3:25   ` [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count " Hui Peng
@ 2026-09-21  7:07     ` Damien Le Moal
  2026-09-21  9:54     ` Niklas Cassel
  2026-09-21 10:03     ` Niklas Cassel
  2 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2026-09-21  7:07 UTC (permalink / raw)
  To: Hui Peng, Niklas Cassel; +Cc: linux-ide, linux-kernel, stable

On 9/21/26 12:25, Hui Peng wrote:
> When an ATA_12, ATA_16, or ATA_32 pass-through command with a
> multi-sector PIO command opcode (ATA_CMD_READ_MULTI,
> ATA_CMD_WRITE_MULTI, ATA_CMD_READ_MULTI_EXT, ATA_CMD_WRITE_MULTI_EXT, or
> ATA_CMD_WRITE_MULTI_FUA_EXT) is submitted via SG_IO on a device where
> multiple-sector mode is not configured (dev->multi_count == 0),
> ata_scsi_pass_thru() currently logs a warning and still dispatches the
> taskfile. When the DRQ interrupt fires, ata_pio_sectors() triggers
> WARN_ON_ONCE(qc->dev->multi_count == 0) and computes
> nsect = min((qc->nbytes - qc->curbytes) / qc->sect_size, 0) = 0, failing
> to transfer any sectors:
> 
>   ata1.00: invalid multi_count 1 ignored
>   WARNING: drivers/ata/libata-sff.c:666 at ata_pio_sectors+0x27d/0x300
>   Call Trace:
>    <IRQ>
>    ata_sff_hsm_move+0x211/0x22e0
>    __ata_sff_port_intr+0x1c8/0x520
>    ata_bmdma_port_intr+0xa1/0x5b0
>    ata_bmdma_interrupt+0x1f5/0x550
> 
> Per ACS-3 section 7.12.7.21, if IDENTIFY DEVICE word 59 bit 8 is cleared
> to zero (multi_count == 0) and a READ MULTIPLE or WRITE MULTIPLE command
> is received without a preceding successful SET MULTIPLE MODE command, the
> device returns command aborted (ABRT). Per the SAT specification, an ABRT
> error translates to the ABORTED COMMAND sense key with NO ADDITIONAL SENSE
> INFORMATION (0x00, 0x00).
> 
> Fail multi-sector taskfile commands in ata_scsi_pass_thru() with
> ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0) when
> dev->multi_count == 0.
> 
> Tested in QEMU against Linux 7.3.0-rc3 (-device ide-cf, where
> dev->multi_count is 0) by issuing ATA_CMD_SET_MULTI (nsect=1) followed
> by an SG_IO ATA_16 ATA_CMD_READ_MULTI command: on the unfixed kernel
> this triggers WARNING: drivers/ata/libata-sff.c:666 in
> ata_pio_sectors(), whereas on the fixed kernel it immediately completes
> with sense key ABORTED_COMMAND (0x0b, 0x00, 0x00) and 0 warnings.
> 
> Fixes: 1dce589c38c3 ("libata passthru: support PIO multi commands")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>

Looks OK.

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

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count is 0
  2026-09-21  3:25   ` [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count " Hui Peng
  2026-09-21  7:07     ` Damien Le Moal
@ 2026-09-21  9:54     ` Niklas Cassel
  2026-09-21 10:03     ` Niklas Cassel
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2026-09-21  9:54 UTC (permalink / raw)
  To: Hui Peng; +Cc: Damien Le Moal, linux-ide, linux-kernel, stable

On Mon, Sep 21, 2026 at 03:25:55AM +0000, Hui Peng wrote:
> When an ATA_12, ATA_16, or ATA_32 pass-through command with a
> multi-sector PIO command opcode (ATA_CMD_READ_MULTI,
> ATA_CMD_WRITE_MULTI, ATA_CMD_READ_MULTI_EXT, ATA_CMD_WRITE_MULTI_EXT, or
> ATA_CMD_WRITE_MULTI_FUA_EXT) is submitted via SG_IO on a device where
> multiple-sector mode is not configured (dev->multi_count == 0),
> ata_scsi_pass_thru() currently logs a warning and still dispatches the
> taskfile. When the DRQ interrupt fires, ata_pio_sectors() triggers
> WARN_ON_ONCE(qc->dev->multi_count == 0) and computes
> nsect = min((qc->nbytes - qc->curbytes) / qc->sect_size, 0) = 0, failing
> to transfer any sectors:
> 
>   ata1.00: invalid multi_count 1 ignored
>   WARNING: drivers/ata/libata-sff.c:666 at ata_pio_sectors+0x27d/0x300
>   Call Trace:
>    <IRQ>
>    ata_sff_hsm_move+0x211/0x22e0
>    __ata_sff_port_intr+0x1c8/0x520
>    ata_bmdma_port_intr+0xa1/0x5b0
>    ata_bmdma_interrupt+0x1f5/0x550
> 
> Per ACS-3 section 7.12.7.21, if IDENTIFY DEVICE word 59 bit 8 is cleared
> to zero (multi_count == 0) and a READ MULTIPLE or WRITE MULTIPLE command
> is received without a preceding successful SET MULTIPLE MODE command, the
> device returns command aborted (ABRT). Per the SAT specification, an ABRT
> error translates to the ABORTED COMMAND sense key with NO ADDITIONAL SENSE
> INFORMATION (0x00, 0x00).
> 
> Fail multi-sector taskfile commands in ata_scsi_pass_thru() with
> ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0) when
> dev->multi_count == 0.
> 
> Tested in QEMU against Linux 7.3.0-rc3 (-device ide-cf, where
> dev->multi_count is 0) by issuing ATA_CMD_SET_MULTI (nsect=1) followed
> by an SG_IO ATA_16 ATA_CMD_READ_MULTI command: on the unfixed kernel
> this triggers WARNING: drivers/ata/libata-sff.c:666 in
> ata_pio_sectors(), whereas on the fixed kernel it immediately completes
> with sense key ABORTED_COMMAND (0x0b, 0x00, 0x00) and 0 warnings.

Testing information is usually placed after "---" so that it does not
get included in the commit message when applied using git am.

I can drop this when applying, no need to send a v3.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count is 0
  2026-09-21  3:25   ` [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count " Hui Peng
  2026-09-21  7:07     ` Damien Le Moal
  2026-09-21  9:54     ` Niklas Cassel
@ 2026-09-21 10:03     ` Niklas Cassel
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2026-09-21 10:03 UTC (permalink / raw)
  To: Damien Le Moal, Hui Peng; +Cc: linux-ide, linux-kernel, stable

On Mon, 21 Sep 2026 03:25:55 +0000, Hui Peng wrote:
> When an ATA_12, ATA_16, or ATA_32 pass-through command with a
> multi-sector PIO command opcode (ATA_CMD_READ_MULTI,
> ATA_CMD_WRITE_MULTI, ATA_CMD_READ_MULTI_EXT, ATA_CMD_WRITE_MULTI_EXT, or
> ATA_CMD_WRITE_MULTI_FUA_EXT) is submitted via SG_IO on a device where
> multiple-sector mode is not configured (dev->multi_count == 0),
> ata_scsi_pass_thru() currently logs a warning and still dispatches the
> taskfile. When the DRQ interrupt fires, ata_pio_sectors() triggers
> WARN_ON_ONCE(qc->dev->multi_count == 0) and computes
> nsect = min((qc->nbytes - qc->curbytes) / qc->sect_size, 0) = 0, failing
> to transfer any sectors:
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: libata-scsi: abort multi-sector pass-through commands when multi_count is 0
      https://git.kernel.org/libata/linux/c/3bab8c7e

Kind regards,
Niklas


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-21 10:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 22:26 [PATCH] ata: libata-scsi: reject multi-sector taskfile commands when dev->multi_count is 0 Hui Peng
2026-09-20  4:32 ` Damien Le Moal
2026-09-21  3:25   ` [PATCH v2] ata: libata-scsi: abort multi-sector pass-through commands when multi_count " Hui Peng
2026-09-21  7:07     ` Damien Le Moal
2026-09-21  9:54     ` Niklas Cassel
2026-09-21 10:03     ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®