From ec87f206d708191abdd332fdfd48fc5b36da083c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 8 Feb 2017 14:51:33 +0100 Subject: cirrus: replace debug printf with trace points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gerd Hoffmann Reviewed-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé Message-id: 1486561893-26470-2-git-send-email-kraxel@redhat.com --- hw/display/cirrus_vga.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'hw/display/cirrus_vga.c') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 16f27e8ac5..b272a70d60 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -28,6 +28,7 @@ */ #include "qemu/osdep.h" #include "qapi/error.h" +#include "trace.h" #include "hw/hw.h" #include "hw/pci/pci.h" #include "ui/console.h" @@ -1852,12 +1853,14 @@ static uint8_t cirrus_mmio_blt_read(CirrusVGAState * s, unsigned address) break; } + trace_vga_cirrus_write_blt(address, value); return (uint8_t) value; } static void cirrus_mmio_blt_write(CirrusVGAState * s, unsigned address, uint8_t value) { + trace_vga_cirrus_write_blt(address, value); switch (address) { case (CIRRUS_MMIO_BLTBGCOLOR + 0): cirrus_vga_write_gr(s, 0x00, value); @@ -2607,9 +2610,7 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr, break; } } -#if defined(DEBUG_VGA) - printf("VGA: read addr=0x%04x data=0x%02x\n", addr, val); -#endif + trace_vga_cirrus_read_io(addr, val); return val; } @@ -2626,9 +2627,7 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val, if (vga_ioport_invalid(s, addr)) { return; } -#ifdef DEBUG_VGA - printf("VGA: write addr=0x%04x data=0x%02x\n", addr, val); -#endif + trace_vga_cirrus_write_io(addr, val); switch (addr) { case 0x3c0: -- cgit 1.4.1 From 95280c31cda79bb1d0968afc7b19a220b3a9d986 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 9 Feb 2017 14:02:20 +0100 Subject: 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 Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Wolfgang Bumiller Reviewed-by: Laurent Vivier Message-id: 1486645341-5010-1-git-send-email-kraxel@redhat.com --- hw/display/cirrus_vga.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) (limited to 'hw/display/cirrus_vga.c') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index b272a70d60..1bcf9a481f 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -684,14 +684,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; } @@ -732,8 +757,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) @@ -832,7 +856,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); -- cgit 1.4.1 From 12e97ec39931e5321645fd483ab761319d48bf16 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 9 Feb 2017 14:02:21 +0100 Subject: Revert "cirrus: allow zero source pitch in pattern fill rops" This reverts commit 5858dd1801883309bdd208d72ddb81c4e9fee30c. Conflicts: hw/display/cirrus_vga.c Cc: Wolfgang Bumiller Cc: Dr. David Alan Gilbert Signed-off-by: Gerd Hoffmann Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Laurent Vivier Message-id: 1486645341-5010-2-git-send-email-kraxel@redhat.com --- hw/display/cirrus_vga.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) (limited to 'hw/display/cirrus_vga.c') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 1bcf9a481f..1deb52070a 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -273,6 +273,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 @@ -291,11 +294,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); @@ -304,10 +304,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; @@ -315,13 +311,7 @@ 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, + if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch, s->cirrus_blt_srcaddr)) { return true; } @@ -716,7 +706,7 @@ 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; } @@ -735,7 +725,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]; @@ -835,7 +825,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, -- cgit 1.4.1