|
 |
9ae3a8 |
From 8ce7227f70248c7f4926124e16baab74c5689841 Mon Sep 17 00:00:00 2001
|
|
|
9ae3a8 |
From: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
9ae3a8 |
Date: Fri, 10 Feb 2017 08:30:13 +0100
|
|
|
9ae3a8 |
Subject: [PATCH 1/3] cirrus: fix patterncopy checks
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
9ae3a8 |
Message-id: <1486715415-3462-2-git-send-email-kraxel@redhat.com>
|
|
|
9ae3a8 |
Patchwork-id: 73775
|
|
|
9ae3a8 |
O-Subject: [virt-devel] [RHEL-7.4 qemu-kvm PATCH 1/3] cirrus: fix patterncopy checks
|
|
|
9ae3a8 |
Bugzilla: 1420492
|
|
|
9ae3a8 |
CVE: CVE-2017-2620/20170221
|
|
|
9ae3a8 |
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
9ae3a8 |
RH-Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
9ae3a8 |
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
The blit_region_is_unsafe checks don't work correctly for the
|
|
|
9ae3a8 |
patterncopy source. It's a fixed-sized region, which doesn't
|
|
|
9ae3a8 |
depend on cirrus_blt_{width,height}. So go do the check in
|
|
|
9ae3a8 |
cirrus_bitblt_common_patterncopy instead, then tell blit_is_unsafe that
|
|
|
9ae3a8 |
it doesn't need to verify the source. Also handle the case where we
|
|
|
9ae3a8 |
blit from cirrus_bitbuf correctly.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
This patch replaces 5858dd1801883309bdd208d72ddb81c4e9fee30c.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Security impact: I think for the most part error on the safe side this
|
|
|
9ae3a8 |
time, refusing blits which should have been allowed.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Only exception is placing the blit source at the end of the video ram,
|
|
|
9ae3a8 |
so cirrus_blt_srcaddr + 256 goes beyond the end of video memory. But
|
|
|
9ae3a8 |
even in that case I'm not fully sure this actually allows read access to
|
|
|
9ae3a8 |
host memory. To trick the commit 5858dd18 security checks one has to
|
|
|
9ae3a8 |
pick very small cirrus_blt_{width,height} values, which in turn implies
|
|
|
9ae3a8 |
only a fraction of the blit source will actually be used.
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
|
|
|
9ae3a8 |
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
9ae3a8 |
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
Upstream: WIP (https://patchwork.ozlabs.org/patch/726080/)
|
|
|
9ae3a8 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
9ae3a8 |
---
|
|
|
9ae3a8 |
hw/display/cirrus_vga.c | 36 ++++++++++++++++++++++++++++++------
|
|
|
9ae3a8 |
1 file changed, 30 insertions(+), 6 deletions(-)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
|
|
|
9ae3a8 |
index e0e39ef..fff9bf0 100644
|
|
|
9ae3a8 |
--- a/hw/display/cirrus_vga.c
|
|
|
9ae3a8 |
+++ b/hw/display/cirrus_vga.c
|
|
|
9ae3a8 |
@@ -678,14 +678,39 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin,
|
|
|
9ae3a8 |
}
|
|
|
9ae3a8 |
}
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
-static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s,
|
|
|
9ae3a8 |
- const uint8_t * src)
|
|
|
9ae3a8 |
+static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc)
|
|
|
9ae3a8 |
{
|
|
|
9ae3a8 |
+ uint32_t patternsize;
|
|
|
9ae3a8 |
uint8_t *dst;
|
|
|
9ae3a8 |
+ uint8_t *src;
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
dst = s->vga.vram_ptr + s->cirrus_blt_dstaddr;
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
- if (blit_is_unsafe(s, false, true)) {
|
|
|
9ae3a8 |
+ if (videosrc) {
|
|
|
9ae3a8 |
+ switch (s->vga.get_bpp(&s->vga)) {
|
|
|
9ae3a8 |
+ case 8:
|
|
|
9ae3a8 |
+ patternsize = 64;
|
|
|
9ae3a8 |
+ break;
|
|
|
9ae3a8 |
+ case 15:
|
|
|
9ae3a8 |
+ case 16:
|
|
|
9ae3a8 |
+ patternsize = 128;
|
|
|
9ae3a8 |
+ break;
|
|
|
9ae3a8 |
+ case 24:
|
|
|
9ae3a8 |
+ case 32:
|
|
|
9ae3a8 |
+ default:
|
|
|
9ae3a8 |
+ patternsize = 256;
|
|
|
9ae3a8 |
+ break;
|
|
|
9ae3a8 |
+ }
|
|
|
9ae3a8 |
+ s->cirrus_blt_srcaddr &= ~(patternsize - 1);
|
|
|
9ae3a8 |
+ if (s->cirrus_blt_srcaddr + patternsize > s->vga.vram_size) {
|
|
|
9ae3a8 |
+ return 0;
|
|
|
9ae3a8 |
+ }
|
|
|
9ae3a8 |
+ src = s->vga.vram_ptr + s->cirrus_blt_srcaddr;
|
|
|
9ae3a8 |
+ } else {
|
|
|
9ae3a8 |
+ src = s->cirrus_bltbuf;
|
|
|
9ae3a8 |
+ }
|
|
|
9ae3a8 |
+
|
|
|
9ae3a8 |
+ if (blit_is_unsafe(s, true, true)) {
|
|
|
9ae3a8 |
return 0;
|
|
|
9ae3a8 |
}
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
@@ -726,8 +751,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
static int cirrus_bitblt_videotovideo_patterncopy(CirrusVGAState * s)
|
|
|
9ae3a8 |
{
|
|
|
9ae3a8 |
- return cirrus_bitblt_common_patterncopy(s, s->vga.vram_ptr +
|
|
|
9ae3a8 |
- (s->cirrus_blt_srcaddr & ~7));
|
|
|
9ae3a8 |
+ return cirrus_bitblt_common_patterncopy(s, true);
|
|
|
9ae3a8 |
}
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
|
|
|
9ae3a8 |
@@ -826,7 +850,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * s)
|
|
|
9ae3a8 |
|
|
|
9ae3a8 |
if (s->cirrus_srccounter > 0) {
|
|
|
9ae3a8 |
if (s->cirrus_blt_mode & CIRRUS_BLTMODE_PATTERNCOPY) {
|
|
|
9ae3a8 |
- cirrus_bitblt_common_patterncopy(s, s->cirrus_bltbuf);
|
|
|
9ae3a8 |
+ cirrus_bitblt_common_patterncopy(s, false);
|
|
|
9ae3a8 |
the_end:
|
|
|
9ae3a8 |
s->cirrus_srccounter = 0;
|
|
|
9ae3a8 |
cirrus_bitblt_reset(s);
|
|
|
9ae3a8 |
--
|
|
|
9ae3a8 |
1.8.3.1
|
|
|
9ae3a8 |
|