9ae3a8
From a0cd3ce8aa79a08cfc41ca5926889ca591ac5cd1 Mon Sep 17 00:00:00 2001
9ae3a8
From: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
Date: Mon, 15 Sep 2014 13:08:21 +0200
9ae3a8
Subject: [PATCH 2/4] vbe: rework sanity checks
9ae3a8
9ae3a8
Message-id: <1410786503-19794-3-git-send-email-kraxel@redhat.com>
9ae3a8
Patchwork-id: 61137
9ae3a8
O-Subject: [RHEL-7.1 qemu-kvm PATCH 2/4] vbe: rework sanity checks
9ae3a8
Bugzilla: 1139118
9ae3a8
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
9ae3a8
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
9ae3a8
9ae3a8
Plug a bunch of holes in the bochs dispi interface parameter checking.
9ae3a8
Add a function doing verification on all registers.  Call that
9ae3a8
unconditionally on every register write.  That way we should catch
9ae3a8
everything, even changing one register affecting the valid range of
9ae3a8
another register.
9ae3a8
9ae3a8
Some of the holes have been added by commit
9ae3a8
e9c6149f6ae6873f14a12eea554925b6aa4c4dec.  Before that commit the
9ae3a8
maximum possible framebuffer (VBE_DISPI_MAX_XRES * VBE_DISPI_MAX_YRES *
9ae3a8
32 bpp) has been smaller than the qemu vga memory (8MB) and the checking
9ae3a8
for VBE_DISPI_MAX_XRES + VBE_DISPI_MAX_YRES + VBE_DISPI_MAX_BPP was ok.
9ae3a8
9ae3a8
Some of the holes have been there forever, such as
9ae3a8
VBE_DISPI_INDEX_X_OFFSET and VBE_DISPI_INDEX_Y_OFFSET register writes
9ae3a8
lacking any verification.
9ae3a8
9ae3a8
Security impact:
9ae3a8
9ae3a8
(1) Guest can make the ui (gtk/vnc/...) use memory rages outside the vga
9ae3a8
frame buffer as source  ->  host memory leak.  Memory isn't leaked to
9ae3a8
the guest but to the vnc client though.
9ae3a8
9ae3a8
(2) Qemu will segfault in case the memory range happens to include
9ae3a8
unmapped areas  ->  Guest can DoS itself.
9ae3a8
9ae3a8
The guest can not modify host memory, so I don't think this can be used
9ae3a8
by the guest to escape.
9ae3a8
9ae3a8
CVE-2014-3615
9ae3a8
9ae3a8
Cc: qemu-stable@nongnu.org
9ae3a8
Cc: secalert@redhat.com
9ae3a8
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
9ae3a8
(cherry picked from commit c1b886c45dc70f247300f549dce9833f3fa2def5)
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 hw/display/vga.c |  154 +++++++++++++++++++++++++++++++++---------------------
9ae3a8
 1 files changed, 95 insertions(+), 59 deletions(-)
9ae3a8
9ae3a8
diff --git a/hw/display/vga.c b/hw/display/vga.c
9ae3a8
index d703d90..de5d63d 100644
9ae3a8
--- a/hw/display/vga.c
9ae3a8
+++ b/hw/display/vga.c
9ae3a8
@@ -579,6 +579,93 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
9ae3a8
     }
9ae3a8
 }
9ae3a8
 
9ae3a8
+/*
9ae3a8
+ * Sanity check vbe register writes.
9ae3a8
+ *
9ae3a8
+ * As we don't have a way to signal errors to the guest in the bochs
9ae3a8
+ * dispi interface we'll go adjust the registers to the closest valid
9ae3a8
+ * value.
9ae3a8
+ */
9ae3a8
+static void vbe_fixup_regs(VGACommonState *s)
9ae3a8
+{
9ae3a8
+    uint16_t *r = s->vbe_regs;
9ae3a8
+    uint32_t bits, linelength, maxy, offset;
9ae3a8
+
9ae3a8
+    if (!(r[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {
9ae3a8
+        /* vbe is turned off -- nothing to do */
9ae3a8
+        return;
9ae3a8
+    }
9ae3a8
+
9ae3a8
+    /* check depth */
9ae3a8
+    switch (r[VBE_DISPI_INDEX_BPP]) {
9ae3a8
+    case 4:
9ae3a8
+    case 8:
9ae3a8
+    case 16:
9ae3a8
+    case 24:
9ae3a8
+    case 32:
9ae3a8
+        bits = r[VBE_DISPI_INDEX_BPP];
9ae3a8
+        break;
9ae3a8
+    case 15:
9ae3a8
+        bits = 16;
9ae3a8
+        break;
9ae3a8
+    default:
9ae3a8
+        bits = r[VBE_DISPI_INDEX_BPP] = 8;
9ae3a8
+        break;
9ae3a8
+    }
9ae3a8
+
9ae3a8
+    /* check width */
9ae3a8
+    r[VBE_DISPI_INDEX_XRES] &= ~7u;
9ae3a8
+    if (r[VBE_DISPI_INDEX_XRES] == 0) {
9ae3a8
+        r[VBE_DISPI_INDEX_XRES] = 8;
9ae3a8
+    }
9ae3a8
+    if (r[VBE_DISPI_INDEX_XRES] > VBE_DISPI_MAX_XRES) {
9ae3a8
+        r[VBE_DISPI_INDEX_XRES] = VBE_DISPI_MAX_XRES;
9ae3a8
+    }
9ae3a8
+    r[VBE_DISPI_INDEX_VIRT_WIDTH] &= ~7u;
9ae3a8
+    if (r[VBE_DISPI_INDEX_VIRT_WIDTH] > VBE_DISPI_MAX_XRES) {
9ae3a8
+        r[VBE_DISPI_INDEX_VIRT_WIDTH] = VBE_DISPI_MAX_XRES;
9ae3a8
+    }
9ae3a8
+    if (r[VBE_DISPI_INDEX_VIRT_WIDTH] < r[VBE_DISPI_INDEX_XRES]) {
9ae3a8
+        r[VBE_DISPI_INDEX_VIRT_WIDTH] = r[VBE_DISPI_INDEX_XRES];
9ae3a8
+    }
9ae3a8
+
9ae3a8
+    /* check height */
9ae3a8
+    linelength = r[VBE_DISPI_INDEX_VIRT_WIDTH] * bits / 8;
9ae3a8
+    maxy = s->vbe_size / linelength;
9ae3a8
+    if (r[VBE_DISPI_INDEX_YRES] == 0) {
9ae3a8
+        r[VBE_DISPI_INDEX_YRES] = 1;
9ae3a8
+    }
9ae3a8
+    if (r[VBE_DISPI_INDEX_YRES] > VBE_DISPI_MAX_YRES) {
9ae3a8
+        r[VBE_DISPI_INDEX_YRES] = VBE_DISPI_MAX_YRES;
9ae3a8
+    }
9ae3a8
+    if (r[VBE_DISPI_INDEX_YRES] > maxy) {
9ae3a8
+        r[VBE_DISPI_INDEX_YRES] = maxy;
9ae3a8
+    }
9ae3a8
+
9ae3a8
+    /* check offset */
9ae3a8
+    if (r[VBE_DISPI_INDEX_X_OFFSET] > VBE_DISPI_MAX_XRES) {
9ae3a8
+        r[VBE_DISPI_INDEX_X_OFFSET] = VBE_DISPI_MAX_XRES;
9ae3a8
+    }
9ae3a8
+    if (r[VBE_DISPI_INDEX_Y_OFFSET] > VBE_DISPI_MAX_YRES) {
9ae3a8
+        r[VBE_DISPI_INDEX_Y_OFFSET] = VBE_DISPI_MAX_YRES;
9ae3a8
+    }
9ae3a8
+    offset = r[VBE_DISPI_INDEX_X_OFFSET] * bits / 8;
9ae3a8
+    offset += r[VBE_DISPI_INDEX_Y_OFFSET] * linelength;
9ae3a8
+    if (offset + r[VBE_DISPI_INDEX_YRES] * linelength > s->vbe_size) {
9ae3a8
+        r[VBE_DISPI_INDEX_Y_OFFSET] = 0;
9ae3a8
+        offset = r[VBE_DISPI_INDEX_X_OFFSET] * bits / 8;
9ae3a8
+        if (offset + r[VBE_DISPI_INDEX_YRES] * linelength > s->vbe_size) {
9ae3a8
+            r[VBE_DISPI_INDEX_X_OFFSET] = 0;
9ae3a8
+            offset = 0;
9ae3a8
+        }
9ae3a8
+    }
9ae3a8
+
9ae3a8
+    /* update vga state */
9ae3a8
+    r[VBE_DISPI_INDEX_VIRT_HEIGHT] = maxy;
9ae3a8
+    s->vbe_line_offset = linelength;
9ae3a8
+    s->vbe_start_addr  = offset / 4;
9ae3a8
+}
9ae3a8
+
9ae3a8
 static uint32_t vbe_ioport_read_index(void *opaque, uint32_t addr)
9ae3a8
 {
9ae3a8
     VGACommonState *s = opaque;
9ae3a8
@@ -648,22 +735,13 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
9ae3a8
             }
9ae3a8
             break;
9ae3a8
         case VBE_DISPI_INDEX_XRES:
9ae3a8
-            if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) {
9ae3a8
-                s->vbe_regs[s->vbe_index] = val;
9ae3a8
-            }
9ae3a8
-            break;
9ae3a8
         case VBE_DISPI_INDEX_YRES:
9ae3a8
-            if (val <= VBE_DISPI_MAX_YRES) {
9ae3a8
-                s->vbe_regs[s->vbe_index] = val;
9ae3a8
-            }
9ae3a8
-            break;
9ae3a8
         case VBE_DISPI_INDEX_BPP:
9ae3a8
-            if (val == 0)
9ae3a8
-                val = 8;
9ae3a8
-            if (val == 4 || val == 8 || val == 15 ||
9ae3a8
-                val == 16 || val == 24 || val == 32) {
9ae3a8
-                s->vbe_regs[s->vbe_index] = val;
9ae3a8
-            }
9ae3a8
+        case VBE_DISPI_INDEX_VIRT_WIDTH:
9ae3a8
+        case VBE_DISPI_INDEX_X_OFFSET:
9ae3a8
+        case VBE_DISPI_INDEX_Y_OFFSET:
9ae3a8
+            s->vbe_regs[s->vbe_index] = val;
9ae3a8
+            vbe_fixup_regs(s);
9ae3a8
             break;
9ae3a8
         case VBE_DISPI_INDEX_BANK:
9ae3a8
             if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4) {
9ae3a8
@@ -680,19 +758,11 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
9ae3a8
                 !(s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {
9ae3a8
                 int h, shift_control;
9ae3a8
 
9ae3a8
-                s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] =
9ae3a8
-                    s->vbe_regs[VBE_DISPI_INDEX_XRES];
9ae3a8
-                s->vbe_regs[VBE_DISPI_INDEX_VIRT_HEIGHT] =
9ae3a8
-                    s->vbe_regs[VBE_DISPI_INDEX_YRES];
9ae3a8
+                s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] = 0;
9ae3a8
                 s->vbe_regs[VBE_DISPI_INDEX_X_OFFSET] = 0;
9ae3a8
                 s->vbe_regs[VBE_DISPI_INDEX_Y_OFFSET] = 0;
9ae3a8
-
9ae3a8
-                if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
9ae3a8
-                    s->vbe_line_offset = s->vbe_regs[VBE_DISPI_INDEX_XRES] >> 1;
9ae3a8
-                else
9ae3a8
-                    s->vbe_line_offset = s->vbe_regs[VBE_DISPI_INDEX_XRES] *
9ae3a8
-                        ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
9ae3a8
-                s->vbe_start_addr = 0;
9ae3a8
+                s->vbe_regs[VBE_DISPI_INDEX_ENABLE] |= VBE_DISPI_ENABLED;
9ae3a8
+                vbe_fixup_regs(s);
9ae3a8
 
9ae3a8
                 /* clear the screen (should be done in BIOS) */
9ae3a8
                 if (!(val & VBE_DISPI_NOCLEARMEM)) {
9ae3a8
@@ -741,40 +811,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
9ae3a8
             s->vbe_regs[s->vbe_index] = val;
9ae3a8
             vga_update_memory_access(s);
9ae3a8
             break;
9ae3a8
-        case VBE_DISPI_INDEX_VIRT_WIDTH:
9ae3a8
-            {
9ae3a8
-                int w, h, line_offset;
9ae3a8
-
9ae3a8
-                if (val < s->vbe_regs[VBE_DISPI_INDEX_XRES])
9ae3a8
-                    return;
9ae3a8
-                w = val;
9ae3a8
-                if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
9ae3a8
-                    line_offset = w >> 1;
9ae3a8
-                else
9ae3a8
-                    line_offset = w * ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
9ae3a8
-                h = s->vbe_size / line_offset;
9ae3a8
-                /* XXX: support weird bochs semantics ? */
9ae3a8
-                if (h < s->vbe_regs[VBE_DISPI_INDEX_YRES])
9ae3a8
-                    return;
9ae3a8
-                s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] = w;
9ae3a8
-                s->vbe_regs[VBE_DISPI_INDEX_VIRT_HEIGHT] = h;
9ae3a8
-                s->vbe_line_offset = line_offset;
9ae3a8
-            }
9ae3a8
-            break;
9ae3a8
-        case VBE_DISPI_INDEX_X_OFFSET:
9ae3a8
-        case VBE_DISPI_INDEX_Y_OFFSET:
9ae3a8
-            {
9ae3a8
-                int x;
9ae3a8
-                s->vbe_regs[s->vbe_index] = val;
9ae3a8
-                s->vbe_start_addr = s->vbe_line_offset * s->vbe_regs[VBE_DISPI_INDEX_Y_OFFSET];
9ae3a8
-                x = s->vbe_regs[VBE_DISPI_INDEX_X_OFFSET];
9ae3a8
-                if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
9ae3a8
-                    s->vbe_start_addr += x >> 1;
9ae3a8
-                else
9ae3a8
-                    s->vbe_start_addr += x * ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
9ae3a8
-                s->vbe_start_addr >>= 2;
9ae3a8
-            }
9ae3a8
-            break;
9ae3a8
         default:
9ae3a8
             break;
9ae3a8
         }
9ae3a8
-- 
9ae3a8
1.7.1
9ae3a8