[pve-devel] applied: [PATCH kvm 1/2] fix #1237: include cirrus follow up fixes

Wolfgang Bumiller w.bumiller at proxmox.com
Thu Feb 2 10:16:14 CET 2017


This fixes CVE-2017-2615 together with pattern-fill
regressions in the ciruss driver. introduced by the fixes
for CVE-2016-9921 and CVE-2016-9922.
---
For the record, cirrus is still the default display in many cases for us,
but that is about to change since it's actually also deprecated.

 ...le-negative-pitch-in-cirrus_invalidate_re.patch |  51 ++++++++++
 ...rus-ignore-source-pitch-value-as-needed-i.patch |  72 ++++++++++++++
 ...ow-zero-source-pitch-in-pattern-fill-rops.patch | 102 ++++++++++++++++++++
 ...003-cirrus-fix-blit-address-mask-handling.patch | 104 +++++++++++++++++++++
 ...cirrus-fix-oob-access-issue-CVE-2017-2615.patch |  50 ++++++++++
 debian/patches/series                              |   5 +
 6 files changed, 384 insertions(+)
 create mode 100644 debian/patches/extra/0001-cirrus-handle-negative-pitch-in-cirrus_invalidate_re.patch
 create mode 100644 debian/patches/extra/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch
 create mode 100644 debian/patches/extra/0002-cirrus-allow-zero-source-pitch-in-pattern-fill-rops.patch
 create mode 100644 debian/patches/extra/0003-cirrus-fix-blit-address-mask-handling.patch
 create mode 100644 debian/patches/extra/0004-cirrus-fix-oob-access-issue-CVE-2017-2615.patch

diff --git a/debian/patches/extra/0001-cirrus-handle-negative-pitch-in-cirrus_invalidate_re.patch b/debian/patches/extra/0001-cirrus-handle-negative-pitch-in-cirrus_invalidate_re.patch
new file mode 100644
index 0000000..a95cf1b
--- /dev/null
+++ b/debian/patches/extra/0001-cirrus-handle-negative-pitch-in-cirrus_invalidate_re.patch
@@ -0,0 +1,51 @@
+From b3ce5aeaacdd0cec5bab1d83ee24bae73b0dd506 Mon Sep 17 00:00:00 2001
+From: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Date: Wed, 25 Jan 2017 14:48:57 +0100
+Subject: [PATCH 1/4] cirrus: handle negative pitch in
+ cirrus_invalidate_region()
+
+cirrus_invalidate_region() calls memory_region_set_dirty()
+on a per-line basis, always ranging from off_begin to
+off_begin+bytesperline. With a negative pitch off_begin
+marks the top most used address and thus we need to do an
+initial shift backwards by a line for negative pitches of
+backward blits, otherwise the first iteration covers the
+line going from the start offset forwards instead of
+backwards.
+Additionally since the start address is inclusive, if we
+shift by a full `bytesperline` we move to the first address
+*not* included in the blit, so we only shift by one less
+than bytesperline.
+
+Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Message-id: 1485352137-29367-1-git-send-email-w.bumiller at proxmox.com
+
+[ kraxel: codestyle fixes ]
+
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+---
+ hw/display/cirrus_vga.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
+index 379910d..0f05e45 100644
+--- a/hw/display/cirrus_vga.c
++++ b/hw/display/cirrus_vga.c
+@@ -661,9 +661,14 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin,
+     int off_cur;
+     int off_cur_end;
+ 
++    if (off_pitch < 0) {
++        off_begin -= bytesperline - 1;
++    }
++
+     for (y = 0; y < lines; y++) {
+ 	off_cur = off_begin;
+ 	off_cur_end = (off_cur + bytesperline) & s->cirrus_addr_mask;
++        assert(off_cur_end >= off_cur);
+         memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
+ 	off_begin += off_pitch;
+     }
+-- 
+2.1.4
+
diff --git a/debian/patches/extra/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch b/debian/patches/extra/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch
new file mode 100644
index 0000000..2b24cdd
--- /dev/null
+++ b/debian/patches/extra/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch
@@ -0,0 +1,72 @@
+From f5dc8e6b503fda1ed87c0f4f53c6d2c76a584872 Mon Sep 17 00:00:00 2001
+From: Bruce Rogers <brogers at suse.com>
+Date: Mon, 9 Jan 2017 13:35:20 -0700
+Subject: [PATCH 1/5] display: cirrus: ignore source pitch value as needed in
+ blit_is_unsafe
+
+Commit 4299b90 added a check which is too broad, given that the source
+pitch value is not required to be initialized for solid fill operations.
+This patch refines the blit_is_unsafe() check to ignore source pitch in
+that case. After applying the above commit as a security patch, we
+noticed the SLES 11 SP4 guest gui failed to initialize properly.
+
+Signed-off-by: Bruce Rogers <brogers at suse.com>
+Message-id: 20170109203520.5619-1-brogers at suse.com
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+---
+ hw/display/cirrus_vga.c | 11 +++++++----
+ 1 file changed, 7 insertions(+), 4 deletions(-)
+
+diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
+index bdb092e..379910d 100644
+--- a/hw/display/cirrus_vga.c
++++ b/hw/display/cirrus_vga.c
+@@ -294,7 +294,7 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s,
+     return false;
+ }
+ 
+-static bool blit_is_unsafe(struct CirrusVGAState *s)
++static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only)
+ {
+     /* should be the case, see cirrus_bitblt_start */
+     assert(s->cirrus_blt_width > 0);
+@@ -308,6 +308,9 @@ static bool blit_is_unsafe(struct CirrusVGAState *s)
+                               s->cirrus_blt_dstaddr & s->cirrus_addr_mask)) {
+         return true;
+     }
++    if (dst_only) {
++        return false;
++    }
+     if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch,
+                               s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) {
+         return true;
+@@ -673,7 +676,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s,
+ 
+     dst = s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask);
+ 
+-    if (blit_is_unsafe(s))
++    if (blit_is_unsafe(s, false))
+         return 0;
+ 
+     (*s->cirrus_rop) (s, dst, src,
+@@ -691,7 +694,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
+ {
+     cirrus_fill_t rop_func;
+ 
+-    if (blit_is_unsafe(s)) {
++    if (blit_is_unsafe(s, true)) {
+         return 0;
+     }
+     rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1];
+@@ -795,7 +798,7 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
+ 
+ static int cirrus_bitblt_videotovideo_copy(CirrusVGAState * s)
+ {
+-    if (blit_is_unsafe(s))
++    if (blit_is_unsafe(s, false))
+         return 0;
+ 
+     return cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->vga.start_addr,
+-- 
+2.1.4
+
diff --git a/debian/patches/extra/0002-cirrus-allow-zero-source-pitch-in-pattern-fill-rops.patch b/debian/patches/extra/0002-cirrus-allow-zero-source-pitch-in-pattern-fill-rops.patch
new file mode 100644
index 0000000..7431baf
--- /dev/null
+++ b/debian/patches/extra/0002-cirrus-allow-zero-source-pitch-in-pattern-fill-rops.patch
@@ -0,0 +1,102 @@
+From cf9c099a7694eb47ded529e1ed40ee8789f32d31 Mon Sep 17 00:00:00 2001
+From: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Date: Tue, 24 Jan 2017 16:35:38 +0100
+Subject: [PATCH 2/4] cirrus: allow zero source pitch in pattern fill rops
+
+The rops used by cirrus_bitblt_common_patterncopy only use
+the destination pitch, so the source pitch shoul allowed to
+be zero and the blit with used for the range check around the
+source address.
+
+Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Message-id: 1485272138-23249-1-git-send-email-w.bumiller at proxmox.com
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+---
+ hw/display/cirrus_vga.c | 27 +++++++++++++++++++--------
+ 1 file changed, 19 insertions(+), 8 deletions(-)
+
+diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
+index 0f05e45..98f089e 100644
+--- a/hw/display/cirrus_vga.c
++++ b/hw/display/cirrus_vga.c
+@@ -272,9 +272,6 @@ static void cirrus_update_memory_access(CirrusVGAState *s);
+ static bool blit_region_is_unsafe(struct CirrusVGAState *s,
+                                   int32_t pitch, int32_t addr)
+ {
+-    if (!pitch) {
+-        return true;
+-    }
+     if (pitch < 0) {
+         int64_t min = addr
+             + ((int64_t)s->cirrus_blt_height-1) * pitch;
+@@ -294,8 +291,11 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s,
+     return false;
+ }
+ 
+-static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only)
++static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only,
++                           bool zero_src_pitch_ok)
+ {
++    int32_t check_pitch;
++
+     /* should be the case, see cirrus_bitblt_start */
+     assert(s->cirrus_blt_width > 0);
+     assert(s->cirrus_blt_height > 0);
+@@ -304,6 +304,10 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only)
+         return true;
+     }
+ 
++    if (!s->cirrus_blt_dstpitch) {
++        return true;
++    }
++
+     if (blit_region_is_unsafe(s, s->cirrus_blt_dstpitch,
+                               s->cirrus_blt_dstaddr & s->cirrus_addr_mask)) {
+         return true;
+@@ -311,7 +315,13 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only)
+     if (dst_only) {
+         return false;
+     }
+-    if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch,
++
++    check_pitch = s->cirrus_blt_srcpitch;
++    if (!zero_src_pitch_ok && !check_pitch) {
++        check_pitch = s->cirrus_blt_width;
++    }
++
++    if (blit_region_is_unsafe(s, check_pitch,
+                               s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) {
+         return true;
+     }
+@@ -681,8 +691,9 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s,
+ 
+     dst = s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask);
+ 
+-    if (blit_is_unsafe(s, false))
++    if (blit_is_unsafe(s, false, true)) {
+         return 0;
++    }
+ 
+     (*s->cirrus_rop) (s, dst, src,
+                       s->cirrus_blt_dstpitch, 0,
+@@ -699,7 +710,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
+ {
+     cirrus_fill_t rop_func;
+ 
+-    if (blit_is_unsafe(s, true)) {
++    if (blit_is_unsafe(s, true, true)) {
+         return 0;
+     }
+     rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1];
+@@ -803,7 +814,7 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
+ 
+ static int cirrus_bitblt_videotovideo_copy(CirrusVGAState * s)
+ {
+-    if (blit_is_unsafe(s, false))
++    if (blit_is_unsafe(s, false, false))
+         return 0;
+ 
+     return cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->vga.start_addr,
+-- 
+2.1.4
+
diff --git a/debian/patches/extra/0003-cirrus-fix-blit-address-mask-handling.patch b/debian/patches/extra/0003-cirrus-fix-blit-address-mask-handling.patch
new file mode 100644
index 0000000..39a410a
--- /dev/null
+++ b/debian/patches/extra/0003-cirrus-fix-blit-address-mask-handling.patch
@@ -0,0 +1,104 @@
+From a173829e6ebd8b2d7f29028f106173ba067c8b8c Mon Sep 17 00:00:00 2001
+From: Gerd Hoffmann <kraxel at redhat.com>
+Date: Wed, 25 Jan 2017 11:09:56 +0100
+Subject: [PATCH 3/4] cirrus: fix blit address mask handling
+
+Apply the cirrus_addr_mask to cirrus_blt_dstaddr and cirrus_blt_srcaddr
+right after assigning them, in cirrus_bitblt_start(), instead of having
+this all over the place in the cirrus code, and missing a few places.
+
+Reported-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+Message-id: 1485338996-17095-1-git-send-email-kraxel at redhat.com
+---
+ hw/display/cirrus_vga.c | 25 ++++++++++++-------------
+ 1 file changed, 12 insertions(+), 13 deletions(-)
+
+diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
+index 98f089e..7db6409 100644
+--- a/hw/display/cirrus_vga.c
++++ b/hw/display/cirrus_vga.c
+@@ -309,7 +309,7 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only,
+     }
+ 
+     if (blit_region_is_unsafe(s, s->cirrus_blt_dstpitch,
+-                              s->cirrus_blt_dstaddr & s->cirrus_addr_mask)) {
++                              s->cirrus_blt_dstaddr)) {
+         return true;
+     }
+     if (dst_only) {
+@@ -322,7 +322,7 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only,
+     }
+ 
+     if (blit_region_is_unsafe(s, check_pitch,
+-                              s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) {
++                              s->cirrus_blt_srcaddr)) {
+         return true;
+     }
+ 
+@@ -689,7 +689,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s,
+ {
+     uint8_t *dst;
+ 
+-    dst = s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask);
++    dst = s->vga.vram_ptr + s->cirrus_blt_dstaddr;
+ 
+     if (blit_is_unsafe(s, false, true)) {
+         return 0;
+@@ -714,7 +714,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
+         return 0;
+     }
+     rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1];
+-    rop_func(s, s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
++    rop_func(s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
+              s->cirrus_blt_dstpitch,
+              s->cirrus_blt_width, s->cirrus_blt_height);
+     cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
+@@ -732,9 +732,8 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
+ 
+ static int cirrus_bitblt_videotovideo_patterncopy(CirrusVGAState * s)
+ {
+-    return cirrus_bitblt_common_patterncopy(s,
+-					    s->vga.vram_ptr + ((s->cirrus_blt_srcaddr & ~7) &
+-                                            s->cirrus_addr_mask));
++    return cirrus_bitblt_common_patterncopy(s, s->vga.vram_ptr +
++                                            (s->cirrus_blt_srcaddr & ~7));
+ }
+ 
+ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
+@@ -788,10 +787,8 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
+     if (notify)
+         graphic_hw_update(s->vga.con);
+ 
+-    (*s->cirrus_rop) (s, s->vga.vram_ptr +
+-		      (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
+-		      s->vga.vram_ptr +
+-		      (s->cirrus_blt_srcaddr & s->cirrus_addr_mask),
++    (*s->cirrus_rop) (s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
++                      s->vga.vram_ptr + s->cirrus_blt_srcaddr,
+ 		      s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
+ 		      s->cirrus_blt_width, s->cirrus_blt_height);
+ 
+@@ -842,8 +839,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * s)
+         } else {
+             /* at least one scan line */
+             do {
+-                (*s->cirrus_rop)(s, s->vga.vram_ptr +
+-                                 (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),
++                (*s->cirrus_rop)(s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
+                                   s->cirrus_bltbuf, 0, 0, s->cirrus_blt_width, 1);
+                 cirrus_invalidate_region(s, s->cirrus_blt_dstaddr, 0,
+                                          s->cirrus_blt_width, 1);
+@@ -962,6 +958,9 @@ static void cirrus_bitblt_start(CirrusVGAState * s)
+     s->cirrus_blt_modeext = s->vga.gr[0x33];
+     blt_rop = s->vga.gr[0x32];
+ 
++    s->cirrus_blt_dstaddr &= s->cirrus_addr_mask;
++    s->cirrus_blt_srcaddr &= s->cirrus_addr_mask;
++
+ #ifdef DEBUG_BITBLT
+     printf("rop=0x%02x mode=0x%02x modeext=0x%02x w=%d h=%d dpitch=%d spitch=%d daddr=0x%08x saddr=0x%08x writemask=0x%02x\n",
+            blt_rop,
+-- 
+2.1.4
+
diff --git a/debian/patches/extra/0004-cirrus-fix-oob-access-issue-CVE-2017-2615.patch b/debian/patches/extra/0004-cirrus-fix-oob-access-issue-CVE-2017-2615.patch
new file mode 100644
index 0000000..fb59147
--- /dev/null
+++ b/debian/patches/extra/0004-cirrus-fix-oob-access-issue-CVE-2017-2615.patch
@@ -0,0 +1,50 @@
+From e3ff618899e53791fdff5dbd3f8fa889a2ed7b1d Mon Sep 17 00:00:00 2001
+From: Li Qiang <liqiang6-s at 360.cn>
+Date: Wed, 1 Feb 2017 09:35:01 +0100
+Subject: [PATCH 4/4] cirrus: fix oob access issue (CVE-2017-2615)
+
+When doing bitblt copy in backward mode, we should minus the
+blt width first just like the adding in the forward mode. This
+can avoid the oob access of the front of vga's vram.
+
+Signed-off-by: Li Qiang <liqiang6-s at 360.cn>
+Reviewed-by: Laszlo Ersek <lersek at redhat.com>
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+Message-id: 1485938101-26602-1-git-send-email-kraxel at redhat.com
+Message-id: 5887254f.863a240a.2c122.5500 at mx.google.com
+
+{ kraxel: with backward blits (negative pitch) addr is the topmost
+          address, so check it as-is against vram size ]
+
+Cc: qemu-stable at nongnu.org
+Cc: P J P <ppandit at redhat.com>
+Cc: Laszlo Ersek <lersek at redhat.com>
+Cc: Paolo Bonzini <pbonzini at redhat.com>
+Cc: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Fixes: d3532a0db02296e687711b8cdc7791924efccea0 (CVE-2014-8106)
+Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
+---
+ hw/display/cirrus_vga.c | 7 +++----
+ 1 file changed, 3 insertions(+), 4 deletions(-)
+
+diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
+index 7db6409..16f27e8 100644
+--- a/hw/display/cirrus_vga.c
++++ b/hw/display/cirrus_vga.c
+@@ -274,10 +274,9 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s,
+ {
+     if (pitch < 0) {
+         int64_t min = addr
+-            + ((int64_t)s->cirrus_blt_height-1) * pitch;
+-        int32_t max = addr
+-            + s->cirrus_blt_width;
+-        if (min < 0 || max > s->vga.vram_size) {
++            + ((int64_t)s->cirrus_blt_height - 1) * pitch
++            - s->cirrus_blt_width;
++        if (min < -1 || addr >= s->vga.vram_size) {
+             return true;
+         }
+     } else {
+-- 
+2.1.4
+
diff --git a/debian/patches/series b/debian/patches/series
index 3ed6f6d..3cc187b 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -83,3 +83,8 @@ extra/CVE-2016-9914-9pfs-add-cleanup-operation-in-FileOperations.patch
 extra/CVE-2016-9915-9pfs-add-cleanup-operation-for-handle-backend-driver.patch
 extra/CVE-2016-9916-9pfs-add-cleanup-operation-for-proxy-backend-driver.patch
 extra/CVE-2016-9921-display-cirrus-check-vga-bits-per-pixel-bpp-value.patch
+extra/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch
+extra/0001-cirrus-handle-negative-pitch-in-cirrus_invalidate_re.patch
+extra/0002-cirrus-allow-zero-source-pitch-in-pattern-fill-rops.patch
+extra/0003-cirrus-fix-blit-address-mask-handling.patch
+extra/0004-cirrus-fix-oob-access-issue-CVE-2017-2615.patch
-- 
2.1.4





More information about the pve-devel mailing list