[PATCH] ata: libata-scsi: reject non-PIO protocols for READ/WRITE MULTIPLE

[PATCH] ata: libata-scsi: reject non-PIO protocols for READ/WRITE MULTIPLE

[Zhao Gongyi]

From: Zhao Gongyi <zhaogongyi@BYTEDANCE.COM>

The commands recognized by is_multi_taskfile() are ATA READ/WRITE
MULTIPLE variants, which transfer data using PIO. ata_scsi_pass_thru()
validates the MULTIPLE_COUNT field for these commands, but accepts any
protocol selected by the ATA PASS-THROUGH CDB.

This allows userspace, for example, to combine WRITE MULTIPLE with the
DMA protocol. With a QEMU PIIX disk, the device enters its PIO data phase
and waits with DRQ asserted while the host waits for DMA completion. The
request remains active until the user-selected command timeout. Since
the disk has a single tag, filesystem writeback and later SG_IO requests
block behind it.

Require the protocol to map to ATA_PROT_PIO when is_multi_taskfile()
recognizes the command. Reject any other protocol as an invalid CDB
field before issuing the taskfile. Keep the check local to the existing
READ/WRITE MULTIPLE handling so pass-through behavior for other ATA
commands is unchanged.

Fixes: 1dce589c38c3 ("libata passthru: support PIO multi commands")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Gongyi <zhaogongyi@BYTEDANCE.COM>
---
Tested on a QEMU PIIX disk with 16-sector transfers and a multiple count
of 16:

- WRITE MULTIPLE with DMA was rejected with ILLEGAL REQUEST/INVALID
  FIELD IN CDB and a field pointer of 1.
- READ MULTIPLE with DMA was rejected with the same sense data.
- WRITE MULTIPLE with PIO data-out completed successfully.
- READ MULTIPLE with PIO data-in completed successfully and returned the
  8192 bytes written by the preceding command.

Both invalid commands completed in less than 1 ms. No ATA timeout or
hung-task warning was reported, and subsequent I/O remained responsive.
The modified libata-scsi.o also builds successfully.

 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 115b75a313..31d822d78e 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 (tf->protocol != ATA_PROT_PIO) {
+			fp = 1 + cdb_offset;
+			goto invalid_fld;
+		}
+
 		if (!dev->multi_count) {
 			ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0);
 			return 1;

base-commit: 69be770908ae57354a41d7e1cc99ab46278d2e86
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] ata: libata-scsi: reject non-PIO protocols for READ/WRITE MULTIPLE

[Niklas Cassel]

Hello Zhao,

On Wed, Oct 28, 2026 at 10:04:32PM +0800, Zhao Gongyi wrote:
> From: Zhao Gongyi <zhaogongyi@BYTEDANCE.COM>
> 
> The commands recognized by is_multi_taskfile() are ATA READ/WRITE
> MULTIPLE variants, which transfer data using PIO. ata_scsi_pass_thru()
> validates the MULTIPLE_COUNT field for these commands, but accepts any
> protocol selected by the ATA PASS-THROUGH CDB.
> 
> This allows userspace, for example, to combine WRITE MULTIPLE with the
> DMA protocol. With a QEMU PIIX disk, the device enters its PIO data phase
> and waits with DRQ asserted while the host waits for DMA completion. The
> request remains active until the user-selected command timeout. Since
> the disk has a single tag, filesystem writeback and later SG_IO requests
> block behind it.
> 
> Require the protocol to map to ATA_PROT_PIO when is_multi_taskfile()
> recognizes the command. Reject any other protocol as an invalid CDB
> field before issuing the taskfile. Keep the check local to the existing
> READ/WRITE MULTIPLE handling so pass-through behavior for other ATA
> commands is unchanged.
> 
> Fixes: 1dce589c38c3 ("libata passthru: support PIO multi commands")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhao Gongyi <zhaogongyi@BYTEDANCE.COM>
> ---
> Tested on a QEMU PIIX disk with 16-sector transfers and a multiple count
> of 16:
> 
> - WRITE MULTIPLE with DMA was rejected with ILLEGAL REQUEST/INVALID
>   FIELD IN CDB and a field pointer of 1.
> - READ MULTIPLE with DMA was rejected with the same sense data.
> - WRITE MULTIPLE with PIO data-out completed successfully.
> - READ MULTIPLE with PIO data-in completed successfully and returned the
>   8192 bytes written by the preceding command.
> 
> Both invalid commands completed in less than 1 ms. No ATA timeout or
> hung-task warning was reported, and subsequent I/O remained responsive.
> The modified libata-scsi.o also builds successfully.
> 
>  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 115b75a313..31d822d78e 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 (tf->protocol != ATA_PROT_PIO) {
> +			fp = 1 + cdb_offset;
> +			goto invalid_fld;
> +		}
> +
>  		if (!dev->multi_count) {
>  			ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND, 0, 0);
>  			return 1;
> 

I'm a bit conflicted about this patch.

There is nothing in SAT specs that requires us to validate the protocol field.
When using normal (non-passthrough commands), the protocol field is initialized
by the regular SCSI to ATA translation done by libata.

For passthrough commands, it comes from the user.

Not only does it come from the user, it requires CAP_SYS_RAWIO:

scsi_cmd_allowed() {
        if (capable(CAP_SYS_RAWIO))
                return true;
    ...


(ATA_12 / ATA_16 / ATA_32 will all return false, so not allowed for a regular
user.)

And... even if the user sends a command with a protocol that is not as per
ACS spec, the command should time out and EH should reset the device.


So... If the user sends a passthrough command with the wrong protocol...
Well, that is on the user, no?
(It also requires CAP_SYS_RAWIO)


If we were to add protocol checks all passthrough commands, then we should
not only check READ/WRITE MULTIPLE, we should check other commands like
WRITE SECTORS (30h) and IDENTIFY DEVICE (ECh) too.

But, I don't think we want that. Damien, thoughts?


Kind regards,
Niklas

Re: [PATCH] ata: libata-scsi: reject non-PIO protocols for READ/WRITE MULTIPLE

[Damien Le Moal]

On 2026/11/10 17:41, Niklas Cassel wrote:
> There is nothing in SAT specs that requires us to validate the protocol field.
> When using normal (non-passthrough commands), the protocol field is initialized
> by the regular SCSI to ATA translation done by libata.
> 
> For passthrough commands, it comes from the user.
> 
> Not only does it come from the user, it requires CAP_SYS_RAWIO:
> 
> scsi_cmd_allowed() {
>         if (capable(CAP_SYS_RAWIO))
>                 return true;
>     ...
> 
> 
> (ATA_12 / ATA_16 / ATA_32 will all return false, so not allowed for a regular
> user.)
> 
> And... even if the user sends a command with a protocol that is not as per
> ACS spec, the command should time out and EH should reset the device.
> 
> 
> So... If the user sends a passthrough command with the wrong protocol...
> Well, that is on the user, no?
> (It also requires CAP_SYS_RAWIO)
> 
> 
> If we were to add protocol checks all passthrough commands, then we should
> not only check READ/WRITE MULTIPLE, we should check other commands like
> WRITE SECTORS (30h) and IDENTIFY DEVICE (ECh) too.
> 
> But, I don't think we want that. Damien, thoughts?

Unless we are talking about a command that libata-scsi emulates, passthrough
commands should not be parsed. We do not want that. So unless the bad SG_IO
command can crash the kernel, we should do nothing if the user sends a bad
command and it fails. We can let the drive fail it, even if that leads to drive
resets etc. As Niklas pointed out, SG_IO is priviledged, so it is not like any
user can create a DOS with bad SG_IO commands.

Also, the use case for this is a little silly in my opinion... Who wants to send
a PIO multi-sector IO using a modern command like ATA 32? libata supports that
if that is all that the drive can do. If the drive can do DMA and/or NCQ, then
better commands will be used without relaying on a PIO passthrough command.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-scsi: reject non-PIO protocols for READ/WRITE MULTIPLE

[Zhao Gongyi]

Hi Niklas and Damien,

On 11/11/26 4:52 PM, Damien Le Moal wrote:
> 
> Unless we are talking about a command that libata-scsi emulates, passthrough
> commands should not be parsed. We do not want that. So unless the bad SG_IO
> command can crash the kernel, we should do nothing if the user sends a bad
> command and it fails. We can let the drive fail it, even if that leads to drive
> resets etc. As Niklas pointed out, SG_IO is priviledged, so it is not like any
> user can create a DOS with bad SG_IO commands.
> 
> Also, the use case for this is a little silly in my opinion... Who wants to send
> a PIO multi-sector IO using a modern command like ATA 32? libata supports that
> if that is all that the drive can do. If the drive can do DMA and/or NCQ, then
> better commands will be used without relaying on a PIO passthrough command.
> 
> 

Thanks for the review and clarification. My testing showed the timeout
and blocked I/O described in the patch, but I do not have evidence of a
kernel crash. I understand the concern about parsing non-emulated ATA
passthrough commands in libata-scsi.

Thanks,
Gongyi