[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