From c42c90e61ea19d5ab42998c57f1dd5ae10d97eed Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 10 Feb 2017 09:08:29 +0100 Subject: [PATCH] more cirrus fixes --- .../0001-cirrus-fix-patterncopy-checks.patch | 100 +++++++++++++++++ ...low-zero-source-pitch-in-pattern-fil.patch | 101 ++++++++++++++++++ debian/patches/series | 2 + 3 files changed, 203 insertions(+) create mode 100644 debian/patches/extra/0001-cirrus-fix-patterncopy-checks.patch create mode 100644 debian/patches/extra/0002-Revert-cirrus-allow-zero-source-pitch-in-pattern-fil.patch diff --git a/debian/patches/extra/0001-cirrus-fix-patterncopy-checks.patch b/debian/patches/extra/0001-cirrus-fix-patterncopy-checks.patch new file mode 100644 index 0000000..d31da17 --- /dev/null +++ b/debian/patches/extra/0001-cirrus-fix-patterncopy-checks.patch @@ -0,0 +1,100 @@ +From 391a9e6fd8c6cf615f2ffe44bb85245df52cc2b6 Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Thu, 9 Feb 2017 14:02:20 +0100 +Subject: [PATCH 1/2] cirrus: fix patterncopy checks + +The blit_region_is_unsafe checks don't work correctly for the +patterncopy source. It's a fixed-sized region, which doesn't +depend on cirrus_blt_{width,height}. So go do the check in +cirrus_bitblt_common_patterncopy instead, then tell blit_is_unsafe that +it doesn't need to verify the source. Also handle the case where we +blit from cirrus_bitbuf correctly. + +This patch replaces 5858dd1801883309bdd208d72ddb81c4e9fee30c. + +Security impact: I think for the most part error on the safe side this +time, refusing blits which should have been allowed. + +Only exception is placing the blit source at the end of the video ram, +so cirrus_blt_srcaddr + 256 goes beyond the end of video memory. But +even in that case I'm not fully sure this actually allows read access to +host memory. To trick the commit 5858dd18 security checks one has to +pick very small cirrus_blt_{width,height} values, which in turn implies +only a fraction of the blit source will actually be used. + +Cc: Wolfgang Bumiller +Cc: Dr. David Alan Gilbert +Signed-off-by: Gerd Hoffmann +--- + hw/display/cirrus_vga.c | 36 ++++++++++++++++++++++++++++++------ + 1 file changed, 30 insertions(+), 6 deletions(-) + +diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c +index 16f27e8..6bd13fc 100644 +--- a/hw/display/cirrus_vga.c ++++ b/hw/display/cirrus_vga.c +@@ -683,14 +683,39 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin, + } + } + +-static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s, +- const uint8_t * src) ++static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc) + { ++ uint32_t patternsize; + uint8_t *dst; ++ uint8_t *src; + + dst = s->vga.vram_ptr + s->cirrus_blt_dstaddr; + +- if (blit_is_unsafe(s, false, true)) { ++ if (videosrc) { ++ switch (s->vga.get_bpp(&s->vga)) { ++ case 8: ++ patternsize = 64; ++ break; ++ case 15: ++ case 16: ++ patternsize = 128; ++ break; ++ case 24: ++ case 32: ++ default: ++ patternsize = 256; ++ break; ++ } ++ s->cirrus_blt_srcaddr &= ~(patternsize - 1); ++ if (s->cirrus_blt_srcaddr + patternsize > s->vga.vram_size) { ++ return 0; ++ } ++ src = s->vga.vram_ptr + s->cirrus_blt_srcaddr; ++ } else { ++ src = s->cirrus_bltbuf; ++ } ++ ++ if (blit_is_unsafe(s, true, true)) { + return 0; + } + +@@ -731,8 +756,7 @@ 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)); ++ return cirrus_bitblt_common_patterncopy(s, true); + } + + static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h) +@@ -831,7 +855,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * s) + + if (s->cirrus_srccounter > 0) { + if (s->cirrus_blt_mode & CIRRUS_BLTMODE_PATTERNCOPY) { +- cirrus_bitblt_common_patterncopy(s, s->cirrus_bltbuf); ++ cirrus_bitblt_common_patterncopy(s, false); + the_end: + s->cirrus_srccounter = 0; + cirrus_bitblt_reset(s); +-- +2.1.4 + diff --git a/debian/patches/extra/0002-Revert-cirrus-allow-zero-source-pitch-in-pattern-fil.patch b/debian/patches/extra/0002-Revert-cirrus-allow-zero-source-pitch-in-pattern-fil.patch new file mode 100644 index 0000000..0b8e6ed --- /dev/null +++ b/debian/patches/extra/0002-Revert-cirrus-allow-zero-source-pitch-in-pattern-fil.patch @@ -0,0 +1,101 @@ +From cba280fe94eaed53952e2997cac1ee2bed6cfdee Mon Sep 17 00:00:00 2001 +From: Wolfgang Bumiller +Date: Fri, 10 Feb 2017 08:34:03 +0100 +Subject: [PATCH 2/2] Revert "cirrus: allow zero source pitch in pattern fill + rops" + +This reverts commit cf9c099a7694eb47ded529e1ed40ee8789f32d31. + +Conflicts: + hw/display/cirrus_vga.c +--- + hw/display/cirrus_vga.c | 29 +++++++++-------------------- + 1 file changed, 9 insertions(+), 20 deletions(-) + +diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c +index 6bd13fc..92e7951 100644 +--- a/hw/display/cirrus_vga.c ++++ b/hw/display/cirrus_vga.c +@@ -272,6 +272,9 @@ 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 +@@ -290,11 +293,8 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s, + return false; + } + +-static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only, +- bool zero_src_pitch_ok) ++static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only) + { +- int32_t check_pitch; +- + /* should be the case, see cirrus_bitblt_start */ + assert(s->cirrus_blt_width > 0); + assert(s->cirrus_blt_height > 0); +@@ -303,10 +303,6 @@ 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)) { + return true; +@@ -314,14 +310,8 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only, + if (dst_only) { + return false; + } +- +- 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)) { ++ if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch, ++ s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) { + return true; + } + +@@ -715,9 +705,8 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc) + src = s->cirrus_bltbuf; + } + +- if (blit_is_unsafe(s, true, true)) { ++ if (blit_is_unsafe(s, true)) + return 0; +- } + + (*s->cirrus_rop) (s, dst, src, + s->cirrus_blt_dstpitch, 0, +@@ -734,7 +723,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) + { + cirrus_fill_t rop_func; + +- if (blit_is_unsafe(s, true, true)) { ++ if (blit_is_unsafe(s, true)) { + return 0; + } + rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1]; +@@ -834,7 +823,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, false)) ++ 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/series b/debian/patches/series index e0c9bf5..eb57186 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -92,3 +92,5 @@ extra/CVE-2016-10028-display-virtio-gpu-3d-check-virgl-capabilities-max_s.patch extra/CVE-2016-10155-watchdog-6300esb-add-exit-function.patch extra/0003-sd-sdhci-check-transfer-mode-register-in-multi-block.patch extra/0004-sd-sdhci-block-count-enable-not-relevant-in-single-b.patch +extra/0001-cirrus-fix-patterncopy-checks.patch +extra/0002-Revert-cirrus-allow-zero-source-pitch-in-pattern-fil.patch -- 2.39.2