# CVE-2016-3710

vga-fix-banked-access-bounds-checking-CVE-2016-3710.patch

# CVE-2016-3712

vga-add-vbe_enabled-helper.patch

vga-factor-out-vga-register-setup.patch

vga-update-vga-register-setup-on-vbe-changes.patch

vga-make-sure-vga-register-setup-for-vbe-stays-intac.patch

From 294a6c15a38669f80920b885287191514ab7d9ff Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <kraxel@redhat.com>

Date: Tue, 26 Apr 2016 14:11:34 +0200

Subject: [PATCH 2/5] vga: add vbe_enabled() helper

Makes code a bit easier to read.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---

 hw/display/vga.c | 13 +++++++++----

 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c

index 69e2554..da1eb4a 100644

--- a/hw/display/vga.c

+++ b/hw/display/vga.c

@@ -166,6 +166,11 @@ static uint32_t expand4[256];

 static uint16_t expand2[256];

 static uint8_t expand4to8[16];

+static inline bool vbe_enabled(VGACommonState *s)

+{

+    return s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED;

+}

+

 static void vga_update_memory_access(VGACommonState *s)

 {

     MemoryRegion *region, *old_region = s->chain4_alias;

@@ -593,7 +598,7 @@ static void vbe_fixup_regs(VGACommonState *s)

     uint16_t *r = s->vbe_regs;

     uint32_t bits, linelength, maxy, offset;

-    if (!(r[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {

+    if (!vbe_enabled(s)) {

         /* vbe is turned off -- nothing to do */

         return;

     }

@@ -1174,7 +1179,7 @@ static void vga_get_offsets(VGACommonState *s,

 {

     uint32_t start_addr, line_offset, line_compare;

-    if (s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED) {

+    if (vbe_enabled(s)) {

         line_offset = s->vbe_line_offset;

         start_addr = s->vbe_start_addr;

         line_compare = 65535;

@@ -1627,7 +1632,7 @@ static int vga_get_bpp(VGACommonState *s)

 {

     int ret;

-    if (s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED) {

+    if (vbe_enabled(s)) {

         ret = s->vbe_regs[VBE_DISPI_INDEX_BPP];

     } else {

         ret = 0;

@@ -1639,7 +1644,7 @@ static void vga_get_resolution(VGACommonState *s, int *pwidth, int *pheight)

 {

     int width, height;

-    if (s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED) {

+    if (vbe_enabled(s)) {

         width = s->vbe_regs[VBE_DISPI_INDEX_XRES];

         height = s->vbe_regs[VBE_DISPI_INDEX_YRES];

     } else {

-- 

1.9.1

From d377918c23d85f64d01914b43bfabc0a46fe974a Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <kraxel@redhat.com>

Date: Tue, 26 Apr 2016 15:24:18 +0200

Subject: [PATCH 3/5] vga: factor out vga register setup

When enabling vbe mode qemu will setup a bunch of vga registers to make

sure the vga emulation operates in correct mode for a linear

framebuffer.  Move that code to a separate function so we can call it

from other places too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---

 hw/display/vga.c | 79 +++++++++++++++++++++++++++++++-------------------------

 1 file changed, 44 insertions(+), 35 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c

index da1eb4a..cf5f97e 100644

--- a/hw/display/vga.c

+++ b/hw/display/vga.c

@@ -673,6 +673,49 @@ static void vbe_fixup_regs(VGACommonState *s)

     s->vbe_start_addr  = offset / 4;

 }

+/* we initialize the VGA graphic mode */

+static void vbe_update_vgaregs(VGACommonState *s)

+{

+    int h, shift_control;

+

+    if (!vbe_enabled(s)) {

+        /* vbe is turned off -- nothing to do */

+        return;

+    }

+

+    /* graphic mode + memory map 1 */

+    s->gr[VGA_GFX_MISC] = (s->gr[VGA_GFX_MISC] & ~0x0c) | 0x04 |

+        VGA_GR06_GRAPHICS_MODE;

+    s->cr[VGA_CRTC_MODE] |= 3; /* no CGA modes */

+    s->cr[VGA_CRTC_OFFSET] = s->vbe_line_offset >> 3;

+    /* width */

+    s->cr[VGA_CRTC_H_DISP] =

+        (s->vbe_regs[VBE_DISPI_INDEX_XRES] >> 3) - 1;

+    /* height (only meaningful if < 1024) */

+    h = s->vbe_regs[VBE_DISPI_INDEX_YRES] - 1;

+    s->cr[VGA_CRTC_V_DISP_END] = h;

+    s->cr[VGA_CRTC_OVERFLOW] = (s->cr[VGA_CRTC_OVERFLOW] & ~0x42) |

+        ((h >> 7) & 0x02) | ((h >> 3) & 0x40);

+    /* line compare to 1023 */

+    s->cr[VGA_CRTC_LINE_COMPARE] = 0xff;

+    s->cr[VGA_CRTC_OVERFLOW] |= 0x10;

+    s->cr[VGA_CRTC_MAX_SCAN] |= 0x40;

+

+    if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4) {

+        shift_control = 0;

+        s->sr[VGA_SEQ_CLOCK_MODE] &= ~8; /* no double line */

+    } else {

+        shift_control = 2;

+        /* set chain 4 mode */

+        s->sr[VGA_SEQ_MEMORY_MODE] |= VGA_SR04_CHN_4M;

+        /* activate all planes */

+        s->sr[VGA_SEQ_PLANE_WRITE] |= VGA_SR02_ALL_PLANES;

+    }

+    s->gr[VGA_GFX_MODE] = (s->gr[VGA_GFX_MODE] & ~0x60) |

+        (shift_control << 5);

+    s->cr[VGA_CRTC_MAX_SCAN] &= ~0x9f; /* no double scan */

+}

+

 static uint32_t vbe_ioport_read_index(void *opaque, uint32_t addr)

 {

     VGACommonState *s = opaque;

@@ -759,53 +802,19 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)

         case VBE_DISPI_INDEX_ENABLE:

             if ((val & VBE_DISPI_ENABLED) &&

                 !(s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {

-                int h, shift_control;

                 s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] = 0;

                 s->vbe_regs[VBE_DISPI_INDEX_X_OFFSET] = 0;

                 s->vbe_regs[VBE_DISPI_INDEX_Y_OFFSET] = 0;

                 s->vbe_regs[VBE_DISPI_INDEX_ENABLE] |= VBE_DISPI_ENABLED;

                 vbe_fixup_regs(s);

+                vbe_update_vgaregs(s);

                 /* clear the screen (should be done in BIOS) */

                 if (!(val & VBE_DISPI_NOCLEARMEM)) {

                     memset(s->vram_ptr, 0,

                            s->vbe_regs[VBE_DISPI_INDEX_YRES] * s->vbe_line_offset);

                 }

-

-                /* we initialize the VGA graphic mode (should be done

-                   in BIOS) */

-                /* graphic mode + memory map 1 */

-                s->gr[VGA_GFX_MISC] = (s->gr[VGA_GFX_MISC] & ~0x0c) | 0x04 |

-                    VGA_GR06_GRAPHICS_MODE;

-                s->cr[VGA_CRTC_MODE] |= 3; /* no CGA modes */

-                s->cr[VGA_CRTC_OFFSET] = s->vbe_line_offset >> 3;

-                /* width */

-                s->cr[VGA_CRTC_H_DISP] =

-                    (s->vbe_regs[VBE_DISPI_INDEX_XRES] >> 3) - 1;

-                /* height (only meaningful if < 1024) */

-                h = s->vbe_regs[VBE_DISPI_INDEX_YRES] - 1;

-                s->cr[VGA_CRTC_V_DISP_END] = h;

-                s->cr[VGA_CRTC_OVERFLOW] = (s->cr[VGA_CRTC_OVERFLOW] & ~0x42) |

-                    ((h >> 7) & 0x02) | ((h >> 3) & 0x40);

-                /* line compare to 1023 */

-                s->cr[VGA_CRTC_LINE_COMPARE] = 0xff;

-                s->cr[VGA_CRTC_OVERFLOW] |= 0x10;

-                s->cr[VGA_CRTC_MAX_SCAN] |= 0x40;

-

-                if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4) {

-                    shift_control = 0;

-                    s->sr[VGA_SEQ_CLOCK_MODE] &= ~8; /* no double line */

-                } else {

-                    shift_control = 2;

-                    /* set chain 4 mode */

-                    s->sr[VGA_SEQ_MEMORY_MODE] |= VGA_SR04_CHN_4M;

-                    /* activate all planes */

-                    s->sr[VGA_SEQ_PLANE_WRITE] |= VGA_SR02_ALL_PLANES;

-                }

-                s->gr[VGA_GFX_MODE] = (s->gr[VGA_GFX_MODE] & ~0x60) |

-                    (shift_control << 5);

-                s->cr[VGA_CRTC_MAX_SCAN] &= ~0x9f; /* no double scan */

             } else {

                 /* XXX: the bios should do that */

                 s->bank_offset = 0;

-- 

1.9.1

From 25935fd1ed3c4337aa4b61902ec580e17b130b63 Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <kraxel@redhat.com>

Date: Tue, 26 Apr 2016 08:49:10 +0200

Subject: [PATCH 1/5] vga: fix banked access bounds checking (CVE-2016-3710)

vga allows banked access to video memory using the window at 0xa00000

and it supports a different access modes with different address

calculations.

The VBE bochs extentions support banked access too, using the

VBE_DISPI_INDEX_BANK register.  The code tries to take the different

address calculations into account and applies different limits to

VBE_DISPI_INDEX_BANK depending on the current access mode.

Which is probably effective in stopping misprogramming by accident.

But from a security point of view completely useless as an attacker

can easily change access modes after setting the bank register.

Drop the bogus check, add range checks to vga_mem_{readb,writeb}

instead.

Fixes: CVE-2016-3710

Reported-by: Qinghao Tang <luodalongde@gmail.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---

 hw/display/vga.c | 24 ++++++++++++++++++------

 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c

index 430e7ed..69e2554 100644

--- a/hw/display/vga.c

+++ b/hw/display/vga.c

@@ -197,6 +197,7 @@ static void vga_update_memory_access(VGACommonState *s)

             break;

         }

         base += isa_mem_base;

+        assert(offset + size <= s->vram_size);

         region = g_malloc(sizeof(*region));

         memory_region_init_alias(region, memory_region_owner(&s->vram),

                                  "vga.chain4", &s->vram, offset, size);

@@ -745,11 +746,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)

             vbe_fixup_regs(s);

             break;

         case VBE_DISPI_INDEX_BANK:

-            if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4) {

-              val &= (s->vbe_bank_mask >> 2);

-            } else {

-              val &= s->vbe_bank_mask;

-            }

+            val &= s->vbe_bank_mask;

             s->vbe_regs[s->vbe_index] = val;

             s->bank_offset = (val << 16);

             vga_update_memory_access(s);

@@ -850,13 +847,21 @@ uint32_t vga_mem_readb(VGACommonState *s, hwaddr addr)

     if (s->sr[VGA_SEQ_MEMORY_MODE] & VGA_SR04_CHN_4M) {

         /* chain 4 mode : simplest access */

+        assert(addr < s->vram_size);

         ret = s->vram_ptr[addr];

     } else if (s->gr[VGA_GFX_MODE] & 0x10) {

         /* odd/even mode (aka text mode mapping) */

         plane = (s->gr[VGA_GFX_PLANE_READ] & 2) | (addr & 1);

-        ret = s->vram_ptr[((addr & ~1) << 1) | plane];

+        addr = ((addr & ~1) << 1) | plane;

+        if (addr >= s->vram_size) {

+            return 0xff;

+        }

+        ret = s->vram_ptr[addr];

     } else {

         /* standard VGA latched access */

+        if (addr * sizeof(uint32_t) >= s->vram_size) {

+            return 0xff;

+        }

         s->latch = ((uint32_t *)s->vram_ptr)[addr];

         if (!(s->gr[VGA_GFX_MODE] & 0x08)) {

@@ -913,6 +918,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val)

         plane = addr & 3;

         mask = (1 << plane);

         if (s->sr[VGA_SEQ_PLANE_WRITE] & mask) {

+            assert(addr < s->vram_size);

             s->vram_ptr[addr] = val;

 #ifdef DEBUG_VGA_MEM

             printf("vga: chain4: [0x" TARGET_FMT_plx "]\n", addr);

@@ -926,6 +932,9 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val)

         mask = (1 << plane);

         if (s->sr[VGA_SEQ_PLANE_WRITE] & mask) {

             addr = ((addr & ~1) << 1) | plane;

+            if (addr >= s->vram_size) {

+                return;

+            }

             s->vram_ptr[addr] = val;

 #ifdef DEBUG_VGA_MEM

             printf("vga: odd/even: [0x" TARGET_FMT_plx "]\n", addr);

@@ -999,6 +1008,9 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val)

         mask = s->sr[VGA_SEQ_PLANE_WRITE];

         s->plane_updated |= mask; /* only used to detect font change */

         write_mask = mask16[mask];

+        if (addr * sizeof(uint32_t) >= s->vram_size) {

+            return;

+        }

         ((uint32_t *)s->vram_ptr)[addr] =

             (((uint32_t *)s->vram_ptr)[addr] & ~write_mask) |

             (val & write_mask);

-- 

1.9.1

From f7926c73685c2bc7124265f567bafb502864c5dd Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <kraxel@redhat.com>

Date: Tue, 26 Apr 2016 14:48:06 +0200

Subject: [PATCH 5/5] vga: make sure vga register setup for vbe stays intact

 (CVE-2016-3712).

Call vbe_update_vgaregs() when the guest touches GFX, SEQ or CRT

registers, to make sure the vga registers will always have the

values needed by vbe mode.  This makes sure the sanity checks

applied by vbe_fixup_regs() are effective.

Without this guests can muck with shift_control, can turn on planar

vga modes or text mode emulation while VBE is active, making qemu

take code paths meant for CGA compatibility, but with the very

large display widths and heigts settable using VBE registers.

Which is good for one or another buffer overflow.  Not that

critical as they typically read overflows happening somewhere

in the display code.  So guests can DoS by crashing qemu with a

segfault, but it is probably not possible to break out of the VM.

Fixes: CVE-2016-3712

Reported-by: Zuozhi Fzz <zuozhi.fzz@alibaba-inc.com>

Reported-by: P J P <ppandit@redhat.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---

 hw/display/vga.c | 6 ++++++

 1 file changed, 6 insertions(+)

diff --git a/hw/display/vga.c b/hw/display/vga.c

index 63d1a70..dd61246 100644

--- a/hw/display/vga.c

+++ b/hw/display/vga.c

@@ -166,6 +166,8 @@ static uint32_t expand4[256];

 static uint16_t expand2[256];

 static uint8_t expand4to8[16];

+static void vbe_update_vgaregs(VGACommonState *s);

+

 static inline bool vbe_enabled(VGACommonState *s)

 {

     return s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED;

@@ -513,6 +515,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)

         printf("vga: write SR%x = 0x%02x\n", s->sr_index, val);

 #endif

         s->sr[s->sr_index] = val & sr_mask[s->sr_index];

+        vbe_update_vgaregs(s);

         if (s->sr_index == VGA_SEQ_CLOCK_MODE) {

             s->update_retrace_info(s);

         }

@@ -544,6 +547,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)

         printf("vga: write GR%x = 0x%02x\n", s->gr_index, val);

 #endif

         s->gr[s->gr_index] = val & gr_mask[s->gr_index];

+        vbe_update_vgaregs(s);

         vga_update_memory_access(s);

         break;

     case VGA_CRT_IM:

@@ -562,10 +566,12 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)

             if (s->cr_index == VGA_CRTC_OVERFLOW) {

                 s->cr[VGA_CRTC_OVERFLOW] = (s->cr[VGA_CRTC_OVERFLOW] & ~0x10) |

                     (val & 0x10);

+                vbe_update_vgaregs(s);

             }

             return;

         }

         s->cr[s->cr_index] = val;

+        vbe_update_vgaregs(s);

         switch(s->cr_index) {

         case VGA_CRTC_H_TOTAL:

-- 

1.9.1

From fbcff82cd7998f93556c28dfc63bbbd7b206c8ce Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <kraxel@redhat.com>

Date: Tue, 26 Apr 2016 15:39:22 +0200

Subject: [PATCH 4/5] vga: update vga register setup on vbe changes

Call the new vbe_update_vgaregs() function on vbe configuration

changes, to make sure vga registers are up-to-date.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---

 hw/display/vga.c | 1 +

 1 file changed, 1 insertion(+)

diff --git a/hw/display/vga.c b/hw/display/vga.c

index cf5f97e..63d1a70 100644

--- a/hw/display/vga.c

+++ b/hw/display/vga.c

@@ -792,6 +792,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)

         case VBE_DISPI_INDEX_Y_OFFSET:

             s->vbe_regs[s->vbe_index] = val;

             vbe_fixup_regs(s);

+            vbe_update_vgaregs(s);

             break;

         case VBE_DISPI_INDEX_BANK:

             val &= s->vbe_bank_mask;

-- 

1.9.1