[PATCH] ata: libata-scsi: reject to-device transfers for simulated commands

[PATCH] ata: libata-scsi: reject to-device transfers for simulated commands

[Zhao Gongyi]

ata_scsi_rbuf_fill() emulates commands that only return data to the
initiator (INQUIRY, MODE SENSE, READ CAPACITY, REPORT LUNS and
MAINTENANCE IN).  It builds the reply in a kernel buffer and then copies
it into the request's scatterlist without looking at the transfer
direction.

The scatterlist is built from the user buffer by the passthrough paths
(bsg/SG_IO), which derive the data direction from the transfer lengths
supplied by userspace.  A request can therefore carry a data-in command
while declaring a to-device transfer.  The buffer is then pinned for
reading only.  In the observed crash, an anonymous VMA covered the
buffer address, but its page was not populated; the read-only pin
resolved to the shared zero page.  Copying the reply into that
scatterlist attempted to write to a read-only kernel page:

  BUG: unable to handle page fault for address: ffff888005140038
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0003) - permissions violation
  PGD 7d00067 P4D 7d00067 PUD 7d01067 PMD 80000000050001a1
  RIP: 0010:memcpy_orig+0x68/0x140
  Call Trace:
   sg_copy_buffer+0x1e2/0x250
   ata_scsi_rbuf_fill+0xe6/0x280
   __ata_scsi_queuecmd+0x12e4/0x1580
   scsi_queue_rq+0x1ff7/0x33d0
   blk_execute_rq+0x1c5/0x390
   scsi_bsg_sg_io_fn+0x3bc/0xb00
   bsg_ioctl+0x3ef/0x550

The faulting address falls inside the kernel rodata mapping and matches
empty_zero_page (Kernel rodata 05000000-05dc2fff, empty_zero_page at
physical 0x5140000), which confirms the reply copy attempted to write
to the shared read-only zero page.

Even when the pinned page happens to be writable the reply silently
corrupts the user buffer, since it was not mapped for writing.  Reject
to-device transfers for these commands with an ILLEGAL REQUEST sense
instead of copying the reply into them.  This is the same class of
problem as commit 5da5231bb478 ("libata: if T_LENGTH is zero, dma
direction should be DMA_NONE"), which rejects inconsistent data
directions for ATA pass-through commands.

Reproduced on 7.3.0-rc6 with the syzkaller C reproducer against an
ata_piix disk (/dev/bsg/0:0:0:0): the unpatched kernel panics on every
attempt (4/4), while with this change the request completes with CHECK
CONDITION and the reproducer survives 5/5 runs.

Standard-length CDBs behave the same way.  A correctly directed 6-byte
INQUIRY still completes normally, while a 6-byte INQUIRY and a 10-byte
READ CAPACITY(10) declared as to-device are rejected with CHECK
CONDITION; on the unpatched kernel those same requests silently write
the reply into the caller's page (the transfer has to be page aligned to
take the zero-copy mapping path, otherwise it bounces through a kernel
buffer and nothing is visible to the caller).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zhao Gongyi <zhaogongyi@bytedance.com>
---
 drivers/ata/libata-scsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b3666519b..8b65b896e 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2009,6 +2009,12 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
 	unsigned long flags;
 	unsigned int len;
 
+	/* A data-out request may contain pages pinned without write access. */
+	if (cmd->sc_data_direction == DMA_TO_DEVICE) {
+		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+		return;
+	}
+
 	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
 
 	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] ata: libata-scsi: reject to-device transfers for simulated commands

[Niklas Cassel]

Hello Zhao,

On Tue, Oct 27, 2026 at 05:08:09PM +0800, Zhao Gongyi wrote:
> ata_scsi_rbuf_fill() emulates commands that only return data to the
> initiator (INQUIRY, MODE SENSE, READ CAPACITY, REPORT LUNS and
> MAINTENANCE IN).  It builds the reply in a kernel buffer and then copies
> it into the request's scatterlist without looking at the transfer
> direction.
> 
> The scatterlist is built from the user buffer by the passthrough paths
> (bsg/SG_IO), which derive the data direction from the transfer lengths
> supplied by userspace.  A request can therefore carry a data-in command
> while declaring a to-device transfer.  The buffer is then pinned for
> reading only.  In the observed crash, an anonymous VMA covered the
> buffer address, but its page was not populated; the read-only pin
> resolved to the shared zero page.  Copying the reply into that
> scatterlist attempted to write to a read-only kernel page:
> 
>   BUG: unable to handle page fault for address: ffff888005140038
>   #PF: supervisor write access in kernel mode
>   #PF: error_code(0x0003) - permissions violation
>   PGD 7d00067 P4D 7d00067 PUD 7d01067 PMD 80000000050001a1
>   RIP: 0010:memcpy_orig+0x68/0x140
>   Call Trace:
>    sg_copy_buffer+0x1e2/0x250
>    ata_scsi_rbuf_fill+0xe6/0x280
>    __ata_scsi_queuecmd+0x12e4/0x1580
>    scsi_queue_rq+0x1ff7/0x33d0
>    blk_execute_rq+0x1c5/0x390
>    scsi_bsg_sg_io_fn+0x3bc/0xb00
>    bsg_ioctl+0x3ef/0x550
> 
> The faulting address falls inside the kernel rodata mapping and matches
> empty_zero_page (Kernel rodata 05000000-05dc2fff, empty_zero_page at
> physical 0x5140000), which confirms the reply copy attempted to write
> to the shared read-only zero page.
> 
> Even when the pinned page happens to be writable the reply silently
> corrupts the user buffer, since it was not mapped for writing.  Reject
> to-device transfers for these commands with an ILLEGAL REQUEST sense
> instead of copying the reply into them.  This is the same class of
> problem as commit 5da5231bb478 ("libata: if T_LENGTH is zero, dma
> direction should be DMA_NONE"), which rejects inconsistent data
> directions for ATA pass-through commands.
> 
> Reproduced on 7.3.0-rc6 with the syzkaller C reproducer against an
> ata_piix disk (/dev/bsg/0:0:0:0): the unpatched kernel panics on every
> attempt (4/4), while with this change the request completes with CHECK
> CONDITION and the reproducer survives 5/5 runs.
> 
> Standard-length CDBs behave the same way.  A correctly directed 6-byte
> INQUIRY still completes normally, while a 6-byte INQUIRY and a 10-byte
> READ CAPACITY(10) declared as to-device are rejected with CHECK
> CONDITION; on the unpatched kernel those same requests silently write
> the reply into the caller's page (the transfer has to be page aligned to
> take the zero-copy mapping path, otherwise it bounces through a kernel
> buffer and nothing is visible to the caller).
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM

The Assisted-by tag is deprecated since commit 1e035297ffda ("docs:
remove Assisted-by references").


> Signed-off-by: Zhao Gongyi <zhaogongyi@bytedance.com>
> ---
>  drivers/ata/libata-scsi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index b3666519b..8b65b896e 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2009,6 +2009,12 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	unsigned long flags;
>  	unsigned int len;
>  
> +	/* A data-out request may contain pages pinned without write access. */
> +	if (cmd->sc_data_direction == DMA_TO_DEVICE) {
> +		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
> +		return;
> +	}
> +

This patch does not compile on mainline.

Please rebase on latest Linus' tree.


The fixed call will be:
ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB);



Your patch seems to only patch simulated commands (ata_scsi_rbuf_fill()),
I think you need to patch translated commands too (ata_scsi_translate()),
something like:

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 5cde90f..6fba9da 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2048,6 +2048,19 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
        if (xlat_func(qc))
                goto done;
 
+       /*
+        * A data-out request may contain pages pinned without write access, so
+        * it must not be issued as a device-to-host transfer. Reject it before
+        * issuing, so that a completion handler never rewrites the buffer.
+        */
+       if (unlikely(cmd->sc_data_direction == DMA_TO_DEVICE &&
+                    ata_is_data(qc->tf.protocol) &&
+                    !(qc->tf.flags & ATA_TFLAG_WRITE))) {
+               ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
+                                  INVALID_FIELD_IN_CDB);
+               goto done;
+       }
+
        return ata_scsi_qc_issue(ap, qc);



Rejecting before issuing is important because the ZBC IN and GET PHYSICAL
ELEMENT STATUS completions rewrite the buffer even when the drive aborts
the command.


Kind regards,
Niklas

[PATCH v2] ata: libata-scsi: reject data-in commands with a data-out buffer

[Zhao Gongyi]

ata_scsi_rbuf_fill() copies simulated data-in replies to the SCSI
scatterlist without checking the request direction. SG_IO can supply a
data-in CDB with DMA_TO_DEVICE, in which case the pages may have been
pinned without write access. In the reported case, copying the reply
into the shared zero page caused a kernel page fault.

Translated commands can reach the same kind of mismatch. In particular,
the ZBC IN and GET PHYSICAL ELEMENT STATUS completion handlers rewrite
the receive buffer, including when the device aborts the command.
Checking the direction only in ata_scsi_rbuf_fill() leaves those paths
unprotected.

Reject DMA_TO_DEVICE requests in the simulated-command path. After
translation, reject a DMA_TO_DEVICE request whose ATA taskfile performs
a data-in transfer, before issuing it to the device. Return ILLEGAL
REQUEST / INVALID FIELD IN CDB for both cases.

The original reproducer panicked in ata_scsi_rbuf_fill() on all four
runs with the unpatched kernel. With this patch applied, it completed
without a crash. A diagnostic run confirmed CHECK CONDITION with
ILLEGAL REQUEST / INVALID FIELD IN CDB.

Built an x86_64 defconfig kernel from the current Linus tree and booted it
in QEMU with an ata_piix disk. Normal INQUIRY and READ(10) SG_IO requests
succeeded. Each command declared as SG_DXFER_TO_DEV returned CHECK
CONDITION with ILLEGAL REQUEST / INVALID FIELD IN CDB. No kernel
warnings or oopses appeared. ZBC IN and GET PHYSICAL ELEMENT STATUS
were checked by code inspection; the test disk does not support them.

Suggested-by: Niklas Cassel <cassel@kernel.org>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Link: https://lore.kernel.org/r/20261027090809.94126-1-zhaogongyi@bytedance.com
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Gongyi <zhaogongyi@bytedance.com>
---
Changes since v1:
  - Rebase on Linus' tree and use INVALID_FIELD_IN_CDB with the current
    ata_scsi_set_sense() signature.
  - Reject mismatched translated data-in commands before issuing them,
    as suggested by Niklas Cassel.
  - Remove the deprecated Assisted-by trailer.

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

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb0331..a5170a36a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2048,6 +2048,19 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
 	if (xlat_func(qc))
 		goto done;
 
+	/*
+	 * A data-out request may contain pages pinned without write access.
+	 * Reject a translated data-in command before issuing it, since its
+	 * completion handler may write to the buffer even if the device fails.
+	 */
+	if (unlikely(cmd->sc_data_direction == DMA_TO_DEVICE &&
+		     ata_is_data(qc->tf.protocol) &&
+		     !(qc->tf.flags & ATA_TFLAG_WRITE))) {
+		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
+				   INVALID_FIELD_IN_CDB);
+		goto done;
+	}
+
 	return ata_scsi_qc_issue(ap, qc);
 
 done:
@@ -2079,6 +2092,13 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
 	unsigned long flags;
 	unsigned int len;
 
+	/* A data-out request may contain pages pinned without write access. */
+	if (cmd->sc_data_direction == DMA_TO_DEVICE) {
+		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
+				   INVALID_FIELD_IN_CDB);
+		return;
+	}
+
 	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
 
 	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v2] ata: libata-scsi: reject data-in commands with a data-out buffer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ata: libata-scsi: ata_scsi_translate() direction check misses atapi_xlat()
- [Medium] ata: libata-scsi: missing ascq argument in ata_scsi_set_sense() calls
--

commit 876b457d8b81c6b456e4cdb4d8967b7f266646de
Author: Zhao Gongyi <zhaogongyi@bytedance.com>

ata: libata-scsi: reject data-in commands with a data-out buffer

This commit rejects simulated and translated data-in SCSI commands that are
submitted with a DMA_TO_DEVICE data direction to prevent writing replies into
pages pinned without write access.

Link: https://lore.kernel.org/r/20261027090809.94126-1-zhaogongyi@bytedance.com

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 115b75a313bc5..6f1c0e6769bbf 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1978,6 +1978,19 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	if (xlat_func(qc))
>  		goto done;
>  
> +	/*
> +	 * A data-out request may contain pages pinned without write access.
> +	 * Reject a translated data-in command before issuing it, since its
> +	 * completion handler may write to the buffer even if the device fails.
> +	 */
> +	if (unlikely(cmd->sc_data_direction == DMA_TO_DEVICE &&
> +		     ata_is_data(qc->tf.protocol) &&
> +		     !(qc->tf.flags & ATA_TFLAG_WRITE))) {

[Severity: High]
Does this direction check in ata_scsi_translate() catch ATAPI commands
translated via atapi_xlat()?

In drivers/ata/libata-scsi.c, atapi_xlat() unconditionally sets
ATA_TFLAG_WRITE whenever scmd->sc_data_direction is DMA_TO_DEVICE, without
checking whether the SCSI CDB is a data-in command:

drivers/ata/libata-scsi.c:atapi_xlat() {
	...
	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
	if (scmd->sc_data_direction == DMA_TO_DEVICE) {
		qc->tf.flags |= ATA_TFLAG_WRITE;
	}
	...
}

Because ATA_TFLAG_WRITE is set, the !(qc->tf.flags & ATA_TFLAG_WRITE) check
in ata_scsi_translate() evaluates to false for ATAPI commands.

If userspace submits an SG_IO request to an ATAPI device with
SG_DXFER_TO_DEV (DMA_TO_DEVICE), a non-empty buffer backed by read-only pages
such as the shared zero page, and a standard INQUIRY CDB with allocation
length 0 (cmnd = {0x12, 0, 0, 0, 0, 0}), the device completes the PACKET
command without a data phase and without error.

On completion, atapi_qc_complete() invokes atapi_fixup_inquiry():

drivers/ata/libata-scsi.c:atapi_qc_complete() {
	...
	/* successful completion path */
	if (cmd->cmnd[0] == INQUIRY && (cmd->cmnd[1] & 0x03) == 0)
		atapi_fixup_inquiry(cmd);
	...
}

atapi_fixup_inquiry() then writes modified INQUIRY header bytes into the
command scatterlist via sg_copy_from_buffer():

drivers/ata/libata-scsi.c:atapi_fixup_inquiry() {
	u8 buf[4];

	sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
	if (buf[2] == 0) {
		buf[2] = 0x5;
		buf[3] = 0x32;
	}
	sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
}

Wouldn't this still trigger a kernel page fault when writing to the read-only
DMA_TO_DEVICE scatterlist pages?

> +		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
> +				   INVALID_FIELD_IN_CDB);
> +		goto done;
> +	}
> +
>  	return ata_scsi_qc_issue(ap, qc);
>  
>  done:
> @@ -2009,6 +2022,13 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	unsigned long flags;
>  	unsigned int len;
>  
> +	/* A data-out request may contain pages pinned without write access. */
> +	if (cmd->sc_data_direction == DMA_TO_DEVICE) {
> +		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
> +				   INVALID_FIELD_IN_CDB);

[Severity: Medium]
Does this compile when CONFIG_ATA is enabled?

In drivers/ata/libata-scsi.c, ata_scsi_set_sense() takes five arguments
(dev, cmd, sk, asc, ascq):

void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd,
			u8 sk, u8 asc, u8 ascq)
{
	bool d_sense = (dev->flags & ATA_DFLAG_D_SENSE);

	scsi_build_sense(cmd, d_sense, sk, asc, ascq);
}

And in include/scsi/scsi_proto.h, INVALID_FIELD_IN_CDB is defined as a single
byte constant for asc:

#define INVALID_FIELD_IN_CDB		0x24

Both new calls to ata_scsi_set_sense() in ata_scsi_translate() and
ata_scsi_rbuf_fill() pass only four arguments, omitting the fifth ascq
argument (0x0), which causes a build failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028113903.19569-1-zhaogongyi@bytedance.com?part=1

Re: [PATCH v2] ata: libata-scsi: reject data-in commands with a data-out buffer

[Niklas Cassel]

Hello Zhao,

When sending a V2 (or V3), please do not use In-Reply-To:
https://docs.kernel.org/process/submitting-patches.html#explicit-in-reply-to-headers

It creates an unmanageable forest of references in email clients.


On Wed, Oct 28, 2026 at 11:53:37AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] ata: libata-scsi: ata_scsi_translate() direction check misses atapi_xlat()

This looks like a valid comment, we probably need something like:

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3193,6 +3193,13 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
 
 	qc->complete_fn = atapi_qc_complete;
 
+	if (scmd->sc_data_direction == DMA_TO_DEVICE &&
+	    scmd->cmnd[0] == INQUIRY) {
+		ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST,
+				   INVALID_FIELD_IN_CDB);
+		return 1;
+	}
+
 	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	if (scmd->sc_data_direction == DMA_TO_DEVICE) {
 		qc->tf.flags |= ATA_TFLAG_WRITE;



> - [Medium] ata: libata-scsi: missing ascq argument in ata_scsi_set_sense() calls

This Sashiko comment is a false positive.

Because Sashiko first tries to apply a patch to libata/for-next before
mainline, Sashiko did not realize that ata_scsi_set_sense() has changed in
mainline by the recent SCSI pull request.

Since Linus has pulled the both the SCSI and the libata pull request, I
updated libata for-next to point to mainline, so Sashiko should not give
this false positive comment should not happen again.


Kind regards,
Niklas

Re: [PATCH v2] ata: libata-scsi: reject data-in commands with a data-out buffer

[Damien Le Moal]

On 2026/10/28 18:39, Zhao Gongyi wrote:
> ata_scsi_rbuf_fill() copies simulated data-in replies to the SCSI
> scatterlist without checking the request direction. SG_IO can supply a
> data-in CDB with DMA_TO_DEVICE, in which case the pages may have been
> pinned without write access. In the reported case, copying the reply
> into the shared zero page caused a kernel page fault.

Sure that can happen if the user sets up the SG_IO incorrectly.

> Translated commands can reach the same kind of mismatch. In particular,

No they cannot, as long as the kernel is correct for internally issued commands.
But t is not clear here if you are still talking about commands passed through
SG_IO.

> the ZBC IN and GET PHYSICAL ELEMENT STATUS completion handlers rewrite
> the receive buffer, including when the device aborts the command.
> Checking the direction only in ata_scsi_rbuf_fill() leaves those paths
> unprotected.
> 
> Reject DMA_TO_DEVICE requests in the simulated-command path. After
> translation, reject a DMA_TO_DEVICE request whose ATA taskfile performs
> a data-in transfer, before issuing it to the device. Return ILLEGAL
> REQUEST / INVALID FIELD IN CDB for both cases.

Why is this being checked here and not in the SCSI layer (sg driver and bsg)? It
seems sensible to check this there.

> The original reproducer panicked in ata_scsi_rbuf_fill() on all four
> runs with the unpatched kernel. With this patch applied, it completed
> without a crash. A diagnostic run confirmed CHECK CONDITION with
> ILLEGAL REQUEST / INVALID FIELD IN CDB.
> 
> Built an x86_64 defconfig kernel from the current Linus tree and booted it
> in QEMU with an ata_piix disk. Normal INQUIRY and READ(10) SG_IO requests
> succeeded. Each command declared as SG_DXFER_TO_DEV returned CHECK
> CONDITION with ILLEGAL REQUEST / INVALID FIELD IN CDB. No kernel
> warnings or oopses appeared. ZBC IN and GET PHYSICAL ELEMENT STATUS
> were checked by code inspection; the test disk does not support them.

Test details are not needed in the commit message.

> 
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Link: https://lore.kernel.org/r/20261027090809.94126-1-zhaogongyi@bytedance.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhao Gongyi <zhaogongyi@bytedance.com>
> ---
> Changes since v1:
>   - Rebase on Linus' tree and use INVALID_FIELD_IN_CDB with the current
>     ata_scsi_set_sense() signature.
>   - Reject mismatched translated data-in commands before issuing them,
>     as suggested by Niklas Cassel.
>   - Remove the deprecated Assisted-by trailer.
> 
>  drivers/ata/libata-scsi.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index a61fb0331..a5170a36a 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2048,6 +2048,19 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	if (xlat_func(qc))
>  		goto done;
>  
> +	/*
> +	 * A data-out request may contain pages pinned without write access.
> +	 * Reject a translated data-in command before issuing it, since its
> +	 * completion handler may write to the buffer even if the device fails.
> +	 */
> +	if (unlikely(cmd->sc_data_direction == DMA_TO_DEVICE &&
> +		     ata_is_data(qc->tf.protocol) &&
> +		     !(qc->tf.flags & ATA_TFLAG_WRITE))) {
> +		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
> +				   INVALID_FIELD_IN_CDB);

The CDB may be perfectly correct. The error is in the SG_IO descriptor setup,
NOT in the CDB. So returning invalid field in CDB seems very misleading.
This is not something that SAT defines as far as I know, so we need to find a
sense code that is more informative. Please have a loo at
include/scsi/scsi_sense.h to see if something better exist.



> +		goto done;
> +	}
> +
>  	return ata_scsi_qc_issue(ap, qc);
>  
>  done:
> @@ -2079,6 +2092,13 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	unsigned long flags;
>  	unsigned int len;
>  
> +	/* A data-out request may contain pages pinned without write access. */
> +	if (cmd->sc_data_direction == DMA_TO_DEVICE) {
> +		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST,
> +				   INVALID_FIELD_IN_CDB);
> +		return;
> +	}
> +
>  	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
>  
>  	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] ata: libata-scsi: reject data-in commands with a data-out buffer

[Zhao Gongyi]

Hi Damien,

Thanks for the review.

On 10/28/26 9:59 PM, Damien Le Moal wrote:
> On 2026/10/28 18:39, Zhao Gongyi wrote:
> 
> No they cannot, as long as the kernel is correct for internally issued commands.
> But t is not clear here if you are still talking about commands passed through
> SG_IO.
> 

Yes, by "translated commands" I meant commands submitted through SG_IO
and subsequently translated by libata, not commands issued internally
by the kernel. I will make that explicit and narrow the description to
the affected ZBC IN and GET PHYSICAL ELEMENT STATUS paths.

> 
> Why is this being checked here and not in the SCSI layer (sg driver and bsg)? It
> seems sensible to check this there.
> 

I checked the SG and BSG submission paths. For data-direction purposes,
they derive the request direction from the SG_IO descriptor and otherwise
treat the CDB as opaque. I could not find a generic helper that derives
the transfer direction of arbitrary standard, service-action, ATA
pass-through, and vendor-specific CDBs.

For simulated commands, libata itself performs the CPU-side write. For
the ZBC IN and GET PHYSICAL ELEMENT STATUS translation paths, the actual
ATA data direction is known after xlat_func() has built qc->tf, and their
completion handlers may modify the receive buffer.

This is why I currently think that checking these paths in libata is the
smallest fix suitable for stable. A broader SG/BSG direction-validation
policy may also be useful, but would likely be a separate change.

> 
> Test details are not needed in the commit message.
> 

Agreed. I will remove the detailed validation results from the commit
message and keep them below the --- separator.

> 
> The CDB may be perfectly correct. The error is in the SG_IO descriptor setup,
> NOT in the CDB. So returning invalid field in CDB seems very misleading.
> This is not something that SAT defines as far as I know, so we need to find a
> sense code that is more informative. Please have a loo at
> include/scsi/scsi_sense.h to see if something better exist.
> 

Agreed. INVALID FIELD IN CDB is misleading here. I propose returning
ABORTED COMMAND / DATA PHASE ERROR (4b/00) instead.

Would this approach be acceptable? If so, I will send a v3 with the
clarified description, the shorter commit message, and the updated sense
data.

Thanks,
Gongyi