xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Xen Security Advisory 209 (CVE-2017-2620) - cirrus_bitblt_cputovideo does not check if memory region is safe
@ 2017-02-21 12:00 Xen.org security team
  2017-02-23  9:43 ` Roger Pau Monné
  0 siblings, 1 reply; 4+ messages in thread
From: Xen.org security team @ 2017-02-21 12:00 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 4095 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

            Xen Security Advisory CVE-2017-2620 / XSA-209
                              version 3

   cirrus_bitblt_cputovideo does not check if memory region is safe

UPDATES IN VERSION 3
====================

Public release.

ISSUE DESCRIPTION
=================

In CIRRUS_BLTMODE_MEMSYSSRC mode the bitblit copy routine
cirrus_bitblt_cputovideo fails to check wethehr the specified memory
region is safe.

IMPACT
======

A malicious guest administrator can cause an out of bounds memory
write, very likely exploitable as a privilege escalation.

VULNERABLE SYSTEMS
==================

Versions of qemu shipped with all Xen versions are vulnerable.

Xen systems running on x86 with HVM guests, with the qemu process
running in dom0 are vulnerable.

Only guests provided with the "cirrus" emulated video card can exploit
the vulnerability.  The non-default "stdvga" emulated video card is
not vulnerable.  (With xl the emulated video card is controlled by the
"stdvga=" and "vga=" domain configuration options.)

ARM systems are not vulnerable.  Systems using only PV guests are not
vulnerable.

For VMs whose qemu process is running in a stub domain, a successful
attacker will only gain the privileges of that stubdom, which should
be only over the guest itself.

Both upstream-based versions of qemu (device_model_version="qemu-xen")
and `traditional' qemu (device_model_version="qemu-xen-traditional")
are vulnerable.

MITIGATION
==========

Running only PV guests will avoid the issue.

Running HVM guests with the device model in a stubdomain will mitigate
the issue.

Changing the video card emulation to stdvga (stdvga=1, vga="stdvga",
in the xl domain configuration) will avoid the vulnerability.

CREDITS
=======

This issue was discovered by Gerd Hoffmann of Red Hat.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa209-qemuu.patch       qemu-xen, qemu upstream
(no backport yet)        qemu-xen-traditional

$ sha256sum xsa209*
167af9ed7163fa7cf4abb52f865290ced3163c7684151bdc1324eb5e534faf13  xsa209-qemut.patch
297578aa43c3e6b21333f1b859fd1d3e68aaaae77b3cadbadd20cfeca8426df3  xsa209-qemuu.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches described above (or others which are
substantially similar) is permitted during the embargo, even on
public-facing systems with untrusted guest users and administrators.

However, deployment of the "stdvga" mitigation (changing the video
card emulation to stdvga) is NOT permitted (except where all the
affected systems and VMs are administered and used only by
organisations which are members of the Xen Project Security Issues
Predisclosure List).  Specifically, deployment on public cloud systems
is NOT permitted.  This is because this produces a guest-visible
change which will indicate which component contains the vulnerability.

Additionally, distribution of updated software is prohibited (except
to other members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJYrBl3AAoJEIP+FMlX6CvZ6LMIALETwnX9w8SifkvuYY3jotwp
nQWY8ztJkMnai9X10RN6SeVf2dCpXLhATPuPGORgRiZJEuBaGHEsHa00i63FQBSL
PaOAgzN1GY+u16Ygv2e3vPcN8mO55A6zcFErF2oLsrfdNsG4pJTwn7bMEjZiqSyG
R9xIC6KiA1nojsZO+ynmRvHxFP6epySRayO0PZAGS75LdmEKVxClE3dAeMW77WNv
dAs3Qi14hB5BmdryK5f1STk8r2b3UsN1pbvao8odiEWFaB9tPo273gj5RdfnEV3t
EzTvH37Q3C4YFoTFx8p6fY5ejHNh4AeSyi9yE7lWtKhDZw56UhdfMmYIgDaKpig=
=RBpg
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa209-qemut.patch --]
[-- Type: application/octet-stream, Size: 1937 bytes --]

From: Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH 3/3] cirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

The missing blit width check allows to overflow cirrus_bltbuf,
with the attractive target cirrus_srcptr (current cirrus_bltbuf write
position) being located right after cirrus_bltbuf in CirrusVGAState.

Due to cirrus emulation writing cirrus_bltbuf bytewise the attacker
hasn't full control over cirrus_srcptr though, only one byte can be
changed.  Once the first byte has been modified further writes land
elsewhere.

[ This is CVE-2017-2620 / XSA-209  - Ian Jackson ]

Fixed compilation by removing extra parameter to blit_is_unsafe. -iwj

Reported-by: Gerd Hoffmann <ghoffman@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e6c3893..45facb6 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -900,6 +900,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 {
     int w;
 
+    if (blit_is_unsafe(s)) {
+        return 0;
+    }
+
     s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_MEMSYSSRC;
     s->cirrus_srcptr = &s->cirrus_bltbuf[0];
     s->cirrus_srcptr_end = &s->cirrus_bltbuf[0];
@@ -925,6 +929,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 	}
         s->cirrus_srccounter = s->cirrus_blt_srcpitch * s->cirrus_blt_height;
     }
+
+    /* the blit_is_unsafe call above should catch this */
+    assert(s->cirrus_blt_srcpitch <= CIRRUS_BLTBUFSIZE);
+
     s->cirrus_srcptr = s->cirrus_bltbuf;
     s->cirrus_srcptr_end = s->cirrus_bltbuf + s->cirrus_blt_srcpitch;
     cirrus_update_memory_access(s);

[-- Attachment #3: xsa209-qemuu.patch --]
[-- Type: application/octet-stream, Size: 1934 bytes --]

From: Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH 3/3] cirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

The missing blit width check allows to overflow cirrus_bltbuf,
with the attractive target cirrus_srcptr (current cirrus_bltbuf write
position) being located right after cirrus_bltbuf in CirrusVGAState.

Due to cirrus emulation writing cirrus_bltbuf bytewise the attacker
hasn't full control over cirrus_srcptr though, only one byte can be
changed.  Once the first byte has been modified further writes land
elsewhere.

[ This is CVE-2017-2620 / XSA-209  - Ian Jackson ]

Reported-by: Gerd Hoffmann <ghoffman@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 0e47cf8..a093dc8 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -899,6 +899,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 {
     int w;
 
+    if (blit_is_unsafe(s, true)) {
+        return 0;
+    }
+
     s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_MEMSYSSRC;
     s->cirrus_srcptr = &s->cirrus_bltbuf[0];
     s->cirrus_srcptr_end = &s->cirrus_bltbuf[0];
@@ -924,6 +928,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 	}
         s->cirrus_srccounter = s->cirrus_blt_srcpitch * s->cirrus_blt_height;
     }
+
+    /* the blit_is_unsafe call above should catch this */
+    assert(s->cirrus_blt_srcpitch <= CIRRUS_BLTBUFSIZE);
+
     s->cirrus_srcptr = s->cirrus_bltbuf;
     s->cirrus_srcptr_end = s->cirrus_bltbuf + s->cirrus_blt_srcpitch;
     cirrus_update_memory_access(s);
-- 
1.8.3.1


[-- Attachment #4: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: Xen Security Advisory 209 (CVE-2017-2620) - cirrus_bitblt_cputovideo does not check if memory region is safe
  2017-02-21 12:00 Xen.org security team
@ 2017-02-23  9:43 ` Roger Pau Monné
  2017-02-23  9:59   ` Steven Haigh
  0 siblings, 1 reply; 4+ messages in thread
From: Roger Pau Monné @ 2017-02-23  9:43 UTC (permalink / raw)
  To: Xen.org security team; +Cc: xen-users, xen-announce, oss-security, xen-devel

On Tue, Feb 21, 2017 at 12:00:03PM +0000, Xen.org security team wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
>             Xen Security Advisory CVE-2017-2620 / XSA-209
>                               version 3
> 
>    cirrus_bitblt_cputovideo does not check if memory region is safe
> 
> UPDATES IN VERSION 3
> ====================
> 
> Public release.
> 
> ISSUE DESCRIPTION
> =================
> 
> In CIRRUS_BLTMODE_MEMSYSSRC mode the bitblit copy routine
> cirrus_bitblt_cputovideo fails to check wethehr the specified memory
> region is safe.
> 
> IMPACT
> ======
> 
> A malicious guest administrator can cause an out of bounds memory
> write, very likely exploitable as a privilege escalation.
> 
> VULNERABLE SYSTEMS
> ==================
> 
> Versions of qemu shipped with all Xen versions are vulnerable.
> 
> Xen systems running on x86 with HVM guests, with the qemu process
> running in dom0 are vulnerable.
> 
> Only guests provided with the "cirrus" emulated video card can exploit
> the vulnerability.  The non-default "stdvga" emulated video card is
> not vulnerable.  (With xl the emulated video card is controlled by the
> "stdvga=" and "vga=" domain configuration options.)
> 
> ARM systems are not vulnerable.  Systems using only PV guests are not
> vulnerable.
> 
> For VMs whose qemu process is running in a stub domain, a successful
> attacker will only gain the privileges of that stubdom, which should
> be only over the guest itself.
> 
> Both upstream-based versions of qemu (device_model_version="qemu-xen")
> and `traditional' qemu (device_model_version="qemu-xen-traditional")
> are vulnerable.
> 
> MITIGATION
> ==========
> 
> Running only PV guests will avoid the issue.
> 
> Running HVM guests with the device model in a stubdomain will mitigate
> the issue.
> 
> Changing the video card emulation to stdvga (stdvga=1, vga="stdvga",
> in the xl domain configuration) will avoid the vulnerability.
> 
> CREDITS
> =======
> 
> This issue was discovered by Gerd Hoffmann of Red Hat.
> 
> RESOLUTION
> ==========
> 
> Applying the appropriate attached patch resolves this issue.
> 
> xsa209-qemuu.patch       qemu-xen, qemu upstream
> (no backport yet)        qemu-xen-traditional

It would be nice to mention that (at least on QEMU shipped with 4.7) the
following patch is also needed for the XSA-209 fix to build correctly:

52b7f43c8fa185ab856bcaacda7abc9a6fc07f84
display: cirrus: ignore source pitch value as needed in blit_is_unsafe

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Xen Security Advisory 209 (CVE-2017-2620) - cirrus_bitblt_cputovideo does not check if memory region is safe
  2017-02-23  9:43 ` Roger Pau Monné
@ 2017-02-23  9:59   ` Steven Haigh
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Haigh @ 2017-02-23  9:59 UTC (permalink / raw)
  To: Roger Pau Monné, Xen.org security team
  Cc: xen-users, xen-devel, xen-announce, oss-security


[-- Attachment #1.1.1: Type: text/plain, Size: 2948 bytes --]

On 23/02/17 20:43, Roger Pau Monné wrote:
> On Tue, Feb 21, 2017 at 12:00:03PM +0000, Xen.org security team wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>             Xen Security Advisory CVE-2017-2620 / XSA-209
>>                               version 3
>>
>>    cirrus_bitblt_cputovideo does not check if memory region is safe
>>
>> UPDATES IN VERSION 3
>> ====================
>>
>> Public release.
>>
>> ISSUE DESCRIPTION
>> =================
>>
>> In CIRRUS_BLTMODE_MEMSYSSRC mode the bitblit copy routine
>> cirrus_bitblt_cputovideo fails to check wethehr the specified memory
>> region is safe.
>>
>> IMPACT
>> ======
>>
>> A malicious guest administrator can cause an out of bounds memory
>> write, very likely exploitable as a privilege escalation.
>>
>> VULNERABLE SYSTEMS
>> ==================
>>
>> Versions of qemu shipped with all Xen versions are vulnerable.
>>
>> Xen systems running on x86 with HVM guests, with the qemu process
>> running in dom0 are vulnerable.
>>
>> Only guests provided with the "cirrus" emulated video card can exploit
>> the vulnerability.  The non-default "stdvga" emulated video card is
>> not vulnerable.  (With xl the emulated video card is controlled by the
>> "stdvga=" and "vga=" domain configuration options.)
>>
>> ARM systems are not vulnerable.  Systems using only PV guests are not
>> vulnerable.
>>
>> For VMs whose qemu process is running in a stub domain, a successful
>> attacker will only gain the privileges of that stubdom, which should
>> be only over the guest itself.
>>
>> Both upstream-based versions of qemu (device_model_version="qemu-xen")
>> and `traditional' qemu (device_model_version="qemu-xen-traditional")
>> are vulnerable.
>>
>> MITIGATION
>> ==========
>>
>> Running only PV guests will avoid the issue.
>>
>> Running HVM guests with the device model in a stubdomain will mitigate
>> the issue.
>>
>> Changing the video card emulation to stdvga (stdvga=1, vga="stdvga",
>> in the xl domain configuration) will avoid the vulnerability.
>>
>> CREDITS
>> =======
>>
>> This issue was discovered by Gerd Hoffmann of Red Hat.
>>
>> RESOLUTION
>> ==========
>>
>> Applying the appropriate attached patch resolves this issue.
>>
>> xsa209-qemuu.patch       qemu-xen, qemu upstream
>> (no backport yet)        qemu-xen-traditional
> 
> It would be nice to mention that (at least on QEMU shipped with 4.7) the
> following patch is also needed for the XSA-209 fix to build correctly:
> 
> 52b7f43c8fa185ab856bcaacda7abc9a6fc07f84
> display: cirrus: ignore source pitch value as needed in blit_is_unsafe

I did request that an updated XSA be issued with this patch - as at the
moment, nobody will be able to apply the XSA only patch to any other
version of Xen.

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Xen Security Advisory 209 (CVE-2017-2620) - cirrus_bitblt_cputovideo does not check if memory region is safe
@ 2017-02-23 15:52 Xen.org security team
  0 siblings, 0 replies; 4+ messages in thread
From: Xen.org security team @ 2017-02-23 15:52 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 4410 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

            Xen Security Advisory CVE-2017-2620 / XSA-209
                              version 4

   cirrus_bitblt_cputovideo does not check if memory region is safe

UPDATES IN VERSION 4
====================

Include a prerequisite patch for qemu-upstream, correct statement
regarding the availability of a qemu-traditional patch.

ISSUE DESCRIPTION
=================

In CIRRUS_BLTMODE_MEMSYSSRC mode the bitblit copy routine
cirrus_bitblt_cputovideo fails to check wethehr the specified memory
region is safe.

IMPACT
======

A malicious guest administrator can cause an out of bounds memory
write, very likely exploitable as a privilege escalation.

VULNERABLE SYSTEMS
==================

Versions of qemu shipped with all Xen versions are vulnerable.

Xen systems running on x86 with HVM guests, with the qemu process
running in dom0 are vulnerable.

Only guests provided with the "cirrus" emulated video card can exploit
the vulnerability.  The non-default "stdvga" emulated video card is
not vulnerable.  (With xl the emulated video card is controlled by the
"stdvga=" and "vga=" domain configuration options.)

ARM systems are not vulnerable.  Systems using only PV guests are not
vulnerable.

For VMs whose qemu process is running in a stub domain, a successful
attacker will only gain the privileges of that stubdom, which should
be only over the guest itself.

Both upstream-based versions of qemu (device_model_version="qemu-xen")
and `traditional' qemu (device_model_version="qemu-xen-traditional")
are vulnerable.

MITIGATION
==========

Running only PV guests will avoid the issue.

Running HVM guests with the device model in a stubdomain will mitigate
the issue.

Changing the video card emulation to stdvga (stdvga=1, vga="stdvga",
in the xl domain configuration) will avoid the vulnerability.

CREDITS
=======

This issue was discovered by Gerd Hoffmann of Red Hat.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa209-qemuu/*.patch     qemu-xen, qemu upstream
xsa209-qemut.patch       qemu-xen-traditional

$ sha256sum xsa209* xsa209*/*
167af9ed7163fa7cf4abb52f865290ced3163c7684151bdc1324eb5e534faf13  xsa209-qemut.patch
e698b73d8de24af0fe33968a43561e5e1d094f4caf2443caa447b552677d2683  xsa209-qemuu/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch
50c60e45151ef2265cce4f92b204e9fd75f8bc8952f097e77ab4fe1c1446bc98  xsa209-qemuu/0002-cirrus-add-blit_is_unsafe-call-to-cirrus_bitblt_cput.patch

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches described above (or others which are
substantially similar) is permitted during the embargo, even on
public-facing systems with untrusted guest users and administrators.

However, deployment of the "stdvga" mitigation (changing the video
card emulation to stdvga) is NOT permitted (except where all the
affected systems and VMs are administered and used only by
organisations which are members of the Xen Project Security Issues
Predisclosure List).  Specifically, deployment on public cloud systems
is NOT permitted.  This is because this produces a guest-visible
change which will indicate which component contains the vulnerability.

Additionally, distribution of updated software is prohibited (except
to other members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJYrwN/AAoJEIP+FMlX6CvZQoQIAK9UiN9VwXv1I0E7X1TL2TjE
P9SNXkI5wKiwCq22pbz9pjBO//ia3M5UoxpDMwaMAQzn9bEThHnki8x2njRxIEF7
frxm6B8DpHLCoRHiqgwi018JHLLcSbr+KQrZqBns1j5BfOF0in89A8cgBmQrziyX
bj9853Q8dHSUNW1vi8vZkMacIwxMCg4sBLjSRUoqiWmoyfU6XodRwZ3LoglsofTj
/jk/G5OiitqXDBPzvclPRddQ53xiN9eN3fV8IdG6QpX6F+C2qQVDyS8kAqqFmmm6
Vn6yl9UxrmP0OmvQ5CgUw8GWQoY3OqObjiPgfNUdbN+CLjdhdGfF3kGuYIniqd4=
=I92f
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa209-qemut.patch --]
[-- Type: application/octet-stream, Size: 1937 bytes --]

From: Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH 3/3] cirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

The missing blit width check allows to overflow cirrus_bltbuf,
with the attractive target cirrus_srcptr (current cirrus_bltbuf write
position) being located right after cirrus_bltbuf in CirrusVGAState.

Due to cirrus emulation writing cirrus_bltbuf bytewise the attacker
hasn't full control over cirrus_srcptr though, only one byte can be
changed.  Once the first byte has been modified further writes land
elsewhere.

[ This is CVE-2017-2620 / XSA-209  - Ian Jackson ]

Fixed compilation by removing extra parameter to blit_is_unsafe. -iwj

Reported-by: Gerd Hoffmann <ghoffman@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e6c3893..45facb6 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -900,6 +900,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 {
     int w;
 
+    if (blit_is_unsafe(s)) {
+        return 0;
+    }
+
     s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_MEMSYSSRC;
     s->cirrus_srcptr = &s->cirrus_bltbuf[0];
     s->cirrus_srcptr_end = &s->cirrus_bltbuf[0];
@@ -925,6 +929,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 	}
         s->cirrus_srccounter = s->cirrus_blt_srcpitch * s->cirrus_blt_height;
     }
+
+    /* the blit_is_unsafe call above should catch this */
+    assert(s->cirrus_blt_srcpitch <= CIRRUS_BLTBUFSIZE);
+
     s->cirrus_srcptr = s->cirrus_bltbuf;
     s->cirrus_srcptr_end = s->cirrus_bltbuf + s->cirrus_blt_srcpitch;
     cirrus_update_memory_access(s);

[-- Attachment #3: xsa209-qemuu/0001-display-cirrus-ignore-source-pitch-value-as-needed-i.patch --]
[-- Type: application/octet-stream, Size: 2607 bytes --]

From 52b7f43c8fa185ab856bcaacda7abc9a6fc07f84 Mon Sep 17 00:00:00 2001
From: Bruce Rogers <brogers@suse.com>
Date: Tue, 21 Feb 2017 10:54:38 -0800
Subject: [PATCH 1/2] 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@suse.com>
Message-id: 20170109203520.5619-1-brogers@suse.com
Signed-off-by: Gerd Hoffmann <kraxel@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 7bf3707..34a6900 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -288,7 +288,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);
@@ -302,6 +302,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;
@@ -667,7 +670,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,
@@ -685,7 +688,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];
@@ -784,7 +787,7 @@ static void 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;
 
     cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->vga.start_addr,
-- 
2.1.4


[-- Attachment #4: xsa209-qemuu/0002-cirrus-add-blit_is_unsafe-call-to-cirrus_bitblt_cput.patch --]
[-- Type: application/octet-stream, Size: 2042 bytes --]

From 15268f91fbe75b38a851c458aef74e693d646ea5 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 21 Feb 2017 10:54:59 -0800
Subject: [PATCH 2/2] cirrus: add blit_is_unsafe call to
 cirrus_bitblt_cputovideo

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

The missing blit width check allows to overflow cirrus_bltbuf,
with the attractive target cirrus_srcptr (current cirrus_bltbuf write
position) being located right after cirrus_bltbuf in CirrusVGAState.

Due to cirrus emulation writing cirrus_bltbuf bytewise the attacker
hasn't full control over cirrus_srcptr though, only one byte can be
changed.  Once the first byte has been modified further writes land
elsewhere.

[ This is CVE-2017-2620 / XSA-209  - Ian Jackson ]

Reported-by: Gerd Hoffmann <ghoffman@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 34a6900..5901250 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -865,6 +865,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 {
     int w;
 
+    if (blit_is_unsafe(s, true)) {
+        return 0;
+    }
+
     s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_MEMSYSSRC;
     s->cirrus_srcptr = &s->cirrus_bltbuf[0];
     s->cirrus_srcptr_end = &s->cirrus_bltbuf[0];
@@ -890,6 +894,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 	}
         s->cirrus_srccounter = s->cirrus_blt_srcpitch * s->cirrus_blt_height;
     }
+
+    /* the blit_is_unsafe call above should catch this */
+    assert(s->cirrus_blt_srcpitch <= CIRRUS_BLTBUFSIZE);
+
     s->cirrus_srcptr = s->cirrus_bltbuf;
     s->cirrus_srcptr_end = s->cirrus_bltbuf + s->cirrus_blt_srcpitch;
     cirrus_update_memory_access(s);
-- 
2.1.4


[-- Attachment #5: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-02-23 15:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-23 15:52 Xen Security Advisory 209 (CVE-2017-2620) - cirrus_bitblt_cputovideo does not check if memory region is safe Xen.org security team
  -- strict thread matches above, loose matches on Subject: below --
2017-02-21 12:00 Xen.org security team
2017-02-23  9:43 ` Roger Pau Monné
2017-02-23  9:59   ` Steven Haigh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).