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

[Niklas Cassel <cassel@kernel.org>]

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