Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Mon, 9 May 2016 17:40:04 +0530 (IST)
From: P J P <ppandit@...hat.com>
To: oss security list <oss-security@...ts.openwall.com>
cc: Michael Roth <mdroth@...ux.vnet.ibm.com>,
        Peter Maydell <peter.maydell@...aro.org>,
        Gerd Hoffmann <ghoffman@...hat.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        zuozhi.fzz@...baba-inc.com
Subject: CVE-2016-3712 Qemu: vga: out-of-bounds read and integer overflow
 issues

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

   Hello,

An out-of-bounds read and integer overflow issue was reported in the Qemu 
emulator's VGA module.

Qemu VGA module allows guest to edit certain registers in 'vbe' and 'vga' 
modes. ie. guest could set certain 'VGA' registers while in 'VBE' mode. This 
leads to potential integer overflow or OOB read access issues in Qemu, 
resulting in DoS by crashing the Qemu process on the host. (Moderate)

A privileged guest user could use this flaw to crash the Qemu process on the 
host.

'CVE-2016-3712' has been assigned to this issue by Red Hat Inc. Patches are 
attached herein to help fix this issue.

This issue was discovered and reported by Zuozhi Fzz of Alibaba Inc.

Thank you.
- --
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJXMH4cAAoJEN0TPTL+WwQfJQ8QAIskTPJjmQ5o2OMyIgTrFlTe
suLiD/qRFInd/MpgeICalqVRBzxh5FdOUJWXCoDUbogPLdZ2LmUHBXL/LyjDK01O
R118M3HYUxWGowPf5Jh+ir4/IPSZamTn0LFZAJCrwWW9dmdqcbnoClDxBm6wsDJD
4uzYmYoHogQZ4DVVL8k9kRrQ1yIkftvwoYZCXN6ToikvXcbJxJdDnc5jQ9W/ABGV
fAfJfsG1zbq2fXagfy+ChKJANse525TAKpTmTZXcZWyoE7JrIUFNFIsWWaBpuJdp
yqj+T8EF0bDz2DxJlmlILkpqg48EaEFJlKBg0jlR8/hkNyl1wgEX+Y/C7mgJd2Om
Dipwkk/G4/izUWls+IZijWeZ2Ge1ul4QG+sM0/InnYhTuyhq3Cw8E8Nt+ZOJHBKj
/KOEYYPr7/QEIC41LKVatN2W5ai6mOSkiGD6qIuIvuR3dPhz7qhFZAML/1KAooAs
QOTPxjqxuMvDUm4+KAF598WY+3UFpDeIF0LExc1bhrvEcrjlhC7ypm02d5WaOk26
wkJQ4hJcbHRs/4vp8mMkpTdz8ccjzfbz3GI1GmSsxN5EbdLW4+r8xgGXZ0o0jwpX
JJHtq1wikxab5+rgC/03oDlGcL2AtD7FvDJtcyGEl+5raDguwNrAuKZoF1cBnTVg
MDzQ2/zuFdeJbIWydjL9
=xwOS
-----END PGP SIGNATURE-----
From fd3c136b3e1482cd0ec7285d6bc2a3e6a62c38d7 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@...hat.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@...baba-inc.com>
Reported-by: P J P <ppandit@...hat.com>
Signed-off-by: Gerd Hoffmann <kraxel@...hat.com>
---
 hw/display/vga.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index eeeb9c8..4a55ec6 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -142,6 +142,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;
@@ -484,6 +486,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);
         }
@@ -515,6 +518,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:
@@ -533,10 +537,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.8.3.1


From 2068192dcccd8a80dddfcc8df6164cf9c26e0fc4 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@...hat.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@...hat.com>
---
 hw/display/vga.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index e12f5ac..eeeb9c8 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -763,6 +763,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.8.3.1


From 7fa5c2c5dc9f9bf878c1e8669eb9644d70a71e71 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@...hat.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@...hat.com>
---
 hw/display/vga.c | 78 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 0c1c5b5..e12f5ac 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -644,6 +644,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;
@@ -730,52 +773,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 */
                 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 */
-                /* 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 {
                 s->bank_offset = 0;
             }
-- 
1.8.3.1


From bfa0f151a564a83b5a26f3e917da98674bf3cf62 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@...hat.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@...hat.com>
---
 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 b9191ca..0c1c5b5 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -142,6 +142,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)
 {
     hwaddr base, offset, size;
@@ -564,7 +569,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;
     }
@@ -1058,7 +1063,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;
@@ -1383,7 +1388,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;
@@ -1395,7 +1400,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.8.3.1

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ