CVE-2011-3346: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Buffer overflow in hw/scsi-disk.c in the SCSI subsystem in QEMU before 0.15.2, as used by Xen, might allow local guest users with permission to access the CD-ROM to cause a denial of service (guest crash) via a crafted SAI READ CAPACITY SCSI command.  NOTE: this is only a vulnerability when root has manually modified certain permissions or ACLs.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2014-04-01T06:35Z
Repository: https://github.com/bonzini/qemu
Language: C
Files changed: hw/scsi-disk.c
Commit date: 2011-09-20 12:27:43 +0200
Commit message: scsi-disk: commonize iovec creation between reads and writes

Also, consistently use qiov.size instead of iov.iov_len.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Diff: diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4a60820b1..84e866278 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -106,10 +106,17 @@ static void scsi_cancel_io(SCSIRequest *req)
         bdrv_aio_cancel(r->req.aiocb);
     }
     r->req.aiocb = NULL;
 }
 
+static uint32_t scsi_init_iovec(SCSIDiskReq *r)
+{
+    r->iov.iov_len = MIN(r->sector_count * 512, SCSI_DMA_BUF_SIZE);
+    qemu_iovec_init_external(&r->qiov, &r->iov, 1);
+    return r->qiov.size / 512;
+}
+
 static void scsi_read_complete(void * opaque, int ret)
 {
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     int n;
@@ -123,16 +130,16 @@ static void scsi_read_complete(void * opaque, int ret)
         if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_READ)) {
             return;
         }
     }
 
-    DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->iov.iov_len);
+    DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->qiov.size);
 
-    n = r->iov.iov_len / 512;
+    n = r->qiov.size / 512;
     r->sector += n;
     r->sector_count -= n;
-    scsi_req_data(&r->req, r->iov.iov_len);
+    scsi_req_data(&r->req, r->qiov.size);
 }
 
 static void scsi_flush_complete(void * opaque, int ret)
 {
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
@@ -179,20 +186,14 @@ static void scsi_read_data(SCSIRequest *req)
         DPRINTF("Data transfer direction invalid\n");
         scsi_read_complete(r, -EINVAL);
         return;
     }
 
-    n = r->sector_count;
-    if (n > SCSI_DMA_BUF_SIZE / 512)
-        n = SCSI_DMA_BUF_SIZE / 512;
-
     if (s->tray_open) {
         scsi_read_complete(r, -ENOMEDIUM);
     }
-    r->iov.iov_len = n * 512;
-    qemu_iovec_init_external(&r->qiov, &r->iov, 1);
-
+    n = scsi_init_iovec(r);
     bdrv_acct_start(s->bs, &r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
     r->req.aiocb = bdrv_aio_readv(s->bs, r->sector, &r->qiov, n,
                               scsi_read_complete, r);
     if (r->req.aiocb == NULL) {
         scsi_read_complete(r, -EIO);
@@ -237,11 +238,10 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
 
 static void scsi_write_complete(void * opaque, int ret)
 {
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-    uint32_t len;
     uint32_t n;
 
     if (r->req.aiocb != NULL) {
         r->req.aiocb = NULL;
         bdrv_acct_done(s->bs, &r->acct);
@@ -251,23 +251,19 @@ static void scsi_write_complete(void * opaque, int ret)
         if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_WRITE)) {
             return;
         }
     }
 
-    n = r->iov.iov_len / 512;
+    n = r->qiov.size / 512;
     r->sector += n;
     r->sector_count -= n;
     if (r->sector_count == 0) {
         scsi_req_complete(&r->req, GOOD);
     } else {
-        len = r->sector_count * 512;
-        if (len > SCSI_DMA_BUF_SIZE) {
-            len = SCSI_DMA_BUF_SIZE;
-        }
-        r->iov.iov_len = len;
-        DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, len);
-        scsi_req_data(&r->req, len);
+        scsi_init_iovec(r);
+        DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
+        scsi_req_data(&r->req, r->qiov.size);
     }
 }
 
 static void scsi_write_data(SCSIRequest *req)
 {
@@ -282,25 +278,23 @@ static void scsi_write_data(SCSIRequest *req)
         DPRINTF("Data transfer direction invalid\n");
         scsi_write_complete(r, -EINVAL);
         return;
     }
 
-    n = r->iov.iov_len / 512;
+    n = r->qiov.size / 512;
     if (n) {
         if (s->tray_open) {
             scsi_write_complete(r, -ENOMEDIUM);
         }
-        qemu_iovec_init_external(&r->qiov, &r->iov, 1);
-
         bdrv_acct_start(s->bs, &r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE);
         r->req.aiocb = bdrv_aio_writev(s->bs, r->sector, &r->qiov, n,
-                                   scsi_write_complete, r);
+                                       scsi_write_complete, r);
         if (r->req.aiocb == NULL) {
             scsi_write_complete(r, -ENOMEM);
         }
     } else {
-        /* Invoke completion routine to fetch data from host.  */
+        /* Called for the first time.  Ask the driver to send us more data.  */
         scsi_write_complete(r, 0);
     }
 }
 
 static void scsi_dma_restart_bh(void *opaque)

Vulnerable code:     DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->iov.iov_len);
    n = r->iov.iov_len / 512;
    scsi_req_data(&r->req, r->iov.iov_len);
    n = r->sector_count;
    if (n > SCSI_DMA_BUF_SIZE / 512)
        n = SCSI_DMA_BUF_SIZE / 512;

    r->iov.iov_len = n * 512;
    qemu_iovec_init_external(&r->qiov, &r->iov, 1);

    uint32_t len;
    n = r->iov.iov_len / 512;
        len = r->sector_count * 512;
        if (len > SCSI_DMA_BUF_SIZE) {
            len = SCSI_DMA_BUF_SIZE;
        }
        r->iov.iov_len = len;
        DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, len);
        scsi_req_data(&r->req, len);
    n = r->iov.iov_len / 512;
        qemu_iovec_init_external(&r->qiov, &r->iov, 1);

                                   scsi_write_complete, r);
        /* Invoke completion routine to fetch data from host.  */
Fixed code: static uint32_t scsi_init_iovec(SCSIDiskReq *r)
{
    r->iov.iov_len = MIN(r->sector_count * 512, SCSI_DMA_BUF_SIZE);
    qemu_iovec_init_external(&r->qiov, &r->iov, 1);
    return r->qiov.size / 512;
}

    DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->qiov.size);
    n = r->qiov.size / 512;
    scsi_req_data(&r->req, r->qiov.size);
    n = scsi_init_iovec(r);
    n = r->qiov.size / 512;
        scsi_init_iovec(r);
        DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
        scsi_req_data(&r->req, r->qiov.size);
    n = r->qiov.size / 512;
                                       scsi_write_complete, r);
        /* Called for the first time.  Ask the driver to send us more data.  */