Skip to content

Commit 6e74e53

Browse files
committed
ata: libata-sff: Ensure that we cannot write outside the allocated buffer
reveliofuzzing reported that a SCSI_IOCTL_SEND_COMMAND ioctl with out_len set to 0xd42, SCSI command set to ATA_16 PASS-THROUGH, ATA command set to ATA_NOP, and protocol set to ATA_PROT_PIO, can cause ata_pio_sector() to write outside the allocated buffer, overwriting random memory. While a ATA device is supposed to abort a ATA_NOP command, there does seem to be a bug either in libata-sff or QEMU, where either this status is not set, or the status is cleared before read by ata_sff_hsm_move(). Anyway, that is most likely a separate bug. Looking at __atapi_pio_bytes(), it already has a safety check to ensure that __atapi_pio_bytes() cannot write outside the allocated buffer. Add a similar check to ata_pio_sector(), such that also ata_pio_sector() cannot write outside the allocated buffer. Cc: stable@vger.kernel.org Reported-by: reveliofuzzing <reveliofuzzing@gmail.com> Closes: https://lore.kernel.org/linux-ide/CA+-ZZ_jTgxh3bS7m+KX07_EWckSnW3N2adX3KV63y4g7M4CZ2A@mail.gmail.com/ Link: https://lore.kernel.org/r/20250127154303.15567-2-cassel@kernel.org Signed-off-by: Niklas Cassel <cassel@kernel.org>
1 parent cc77e2c commit 6e74e53

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

drivers/ata/libata-sff.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
601601
{
602602
struct ata_port *ap = qc->ap;
603603
struct page *page;
604-
unsigned int offset;
604+
unsigned int offset, count;
605605

606606
if (!qc->cursg) {
607607
qc->curbytes = qc->nbytes;
@@ -617,25 +617,27 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
617617
page = nth_page(page, (offset >> PAGE_SHIFT));
618618
offset %= PAGE_SIZE;
619619

620-
trace_ata_sff_pio_transfer_data(qc, offset, qc->sect_size);
620+
/* don't overrun current sg */
621+
count = min(qc->cursg->length - qc->cursg_ofs, qc->sect_size);
622+
623+
trace_ata_sff_pio_transfer_data(qc, offset, count);
621624

622625
/*
623626
* Split the transfer when it splits a page boundary. Note that the
624627
* split still has to be dword aligned like all ATA data transfers.
625628
*/
626629
WARN_ON_ONCE(offset % 4);
627-
if (offset + qc->sect_size > PAGE_SIZE) {
630+
if (offset + count > PAGE_SIZE) {
628631
unsigned int split_len = PAGE_SIZE - offset;
629632

630633
ata_pio_xfer(qc, page, offset, split_len);
631-
ata_pio_xfer(qc, nth_page(page, 1), 0,
632-
qc->sect_size - split_len);
634+
ata_pio_xfer(qc, nth_page(page, 1), 0, count - split_len);
633635
} else {
634-
ata_pio_xfer(qc, page, offset, qc->sect_size);
636+
ata_pio_xfer(qc, page, offset, count);
635637
}
636638

637-
qc->curbytes += qc->sect_size;
638-
qc->cursg_ofs += qc->sect_size;
639+
qc->curbytes += count;
640+
qc->cursg_ofs += count;
639641

640642
if (qc->cursg_ofs == qc->cursg->length) {
641643
qc->cursg = sg_next(qc->cursg);

0 commit comments

Comments
 (0)