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