* [Qemu-devel] [PULL 1/4] cirrus: handle negative pitch in cirrus_invalidate_region()
2017-02-02 15:05 [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Gerd Hoffmann
@ 2017-02-02 15:05 ` Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 2/4] cirrus: allow zero source pitch in pattern fill rops Gerd Hoffmann
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2017-02-02 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Wolfgang Bumiller, Gerd Hoffmann
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
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@proxmox.com>
Message-id: 1485352137-29367-1-git-send-email-w.bumiller@proxmox.com
[ kraxel: codestyle fixes ]
Signed-off-by: Gerd Hoffmann <kraxel@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;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 2/4] cirrus: allow zero source pitch in pattern fill rops
2017-02-02 15:05 [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 1/4] cirrus: handle negative pitch in cirrus_invalidate_region() Gerd Hoffmann
@ 2017-02-02 15:05 ` Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 3/4] cirrus: fix blit address mask handling Gerd Hoffmann
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2017-02-02 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Wolfgang Bumiller, Gerd Hoffmann
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
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@proxmox.com>
Message-id: 1485272138-23249-1-git-send-email-w.bumiller@proxmox.com
Signed-off-by: Gerd Hoffmann <kraxel@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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 3/4] cirrus: fix blit address mask handling
2017-02-02 15:05 [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 1/4] cirrus: handle negative pitch in cirrus_invalidate_region() Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 2/4] cirrus: allow zero source pitch in pattern fill rops Gerd Hoffmann
@ 2017-02-02 15:05 ` Gerd Hoffmann
2017-02-02 15:05 ` [Qemu-devel] [PULL 4/4] cirrus: fix oob access issue (CVE-2017-2615) Gerd Hoffmann
2017-02-03 11:36 ` [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Peter Maydell
4 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2017-02-02 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
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@proxmox.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485338996-17095-1-git-send-email-kraxel@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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PULL 4/4] cirrus: fix oob access issue (CVE-2017-2615)
2017-02-02 15:05 [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Gerd Hoffmann
` (2 preceding siblings ...)
2017-02-02 15:05 ` [Qemu-devel] [PULL 3/4] cirrus: fix blit address mask handling Gerd Hoffmann
@ 2017-02-02 15:05 ` Gerd Hoffmann
2017-02-03 11:36 ` [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Peter Maydell
4 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2017-02-02 15:05 UTC (permalink / raw)
To: qemu-devel
Cc: Li Qiang, qemu-stable, P J P, Laszlo Ersek, Paolo Bonzini,
Wolfgang Bumiller, Gerd Hoffmann
From: Li Qiang <liqiang6-s@360.cn>
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@360.cn>
{ kraxel: with backward blits (negative pitch) addr is the topmost
address, so check it as-is against vram size ]
Cc: qemu-stable@nongnu.org
Cc: P J P <ppandit@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Fixes: d3532a0db02296e687711b8cdc7791924efccea0 (CVE-2014-8106)
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485938101-26602-1-git-send-email-kraxel@redhat.com
Reviewed-by: Laszlo Ersek <lersek@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 {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix.
2017-02-02 15:05 [Qemu-devel] [PULL 0/4] cirrus: multiple bugfixes, including CVE-2017-2615 fix Gerd Hoffmann
` (3 preceding siblings ...)
2017-02-02 15:05 ` [Qemu-devel] [PULL 4/4] cirrus: fix oob access issue (CVE-2017-2615) Gerd Hoffmann
@ 2017-02-03 11:36 ` Peter Maydell
4 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2017-02-03 11:36 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers
On 2 February 2017 at 15:05, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> Here is the vga patch queue, finally bringing the cirrus fixes.
>
> Pull v2 fixes the review and message id lines in patch #4.
> No code changes.
>
> please pull,
> Gerd
>
> The following changes since commit a0def594286d9110a6035e02eef558cf3cf5d847:
>
> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-01-30 10:23:20 +0000)
>
> are available in the git repository at:
>
>
> git://git.kraxel.org/qemu tags/pull-vga-20170202-2
>
> for you to fetch changes up to 62d4c6bd5263bb8413a06c80144fc678df6dfb64:
>
> cirrus: fix oob access issue (CVE-2017-2615) (2017-02-02 15:58:23 +0100)
>
> ----------------------------------------------------------------
> cirrus: multiple bugfixes, including CVE-2017-2615 fix.
>
> ----------------------------------------------------------------
> Gerd Hoffmann (1):
> cirrus: fix blit address mask handling
>
> Li Qiang (1):
> cirrus: fix oob access issue (CVE-2017-2615)
>
> Wolfgang Bumiller (2):
> cirrus: handle negative pitch in cirrus_invalidate_region()
> cirrus: allow zero source pitch in pattern fill rops
>
> hw/display/cirrus_vga.c | 64 ++++++++++++++++++++++++++++++-------------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread