* Re: [PATCH] ata: libata-scsi: reject to-device transfers for simulated commands
2026-10-27 9:08 [PATCH] ata: libata-scsi: reject to-device transfers for simulated commands Zhao Gongyi
@ 2026-10-27 21:22 ` Niklas Cassel
0 siblings, 0 replies; 2+ messages in thread
From: Niklas Cassel @ 2026-10-27 21:22 UTC (permalink / raw)
To: Zhao Gongyi; +Cc: dlemoal, linux-ide, linux-kernel
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
^ permalink raw reply [flat|nested] 2+ messages in thread