xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Xen Security Advisory 138 (CVE-2015-5154) - QEMU heap overflow flaw while processing certain ATAPI commands.
@ 2015-07-27 12:03 Xen.org security team
  0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2015-07-27 12:03 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

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

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

            Xen Security Advisory CVE-2015-5154 / XSA-138
                              version 2

   QEMU heap overflow flaw while processing certain ATAPI commands.

UPDATES IN VERSION 2
====================

Public release.

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

The QEMU security team has predisclosed the following advisory:

    A heap overflow flaw was found in the way QEMU's IDE subsystem
    handled I/O buffer access while processing certain ATAPI commands.

    A privileged guest user in a guest with CDROM drive enabled could
    potentially use this flaw to execute arbitrary code on the host
    with the privileges of the host's QEMU process corresponding to
    the guest.

IMPACT
======

An HVM guest which has access to an emulated IDE CDROM device
(e.g. with a device with "devtype=cdrom", or the "cdrom" convenience
alias, in the VBD configuration) can exploit this vulnerability to
take over the qemu process elevating its privilege to that of the qemu
process.

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

All Xen systems running x86 HVM guests without stubdomains which have
been configured with an emulated CD-ROM driver model are vulnerable.

Systems using qemu-dm stubdomain device models (for example, by
specifying "device_model_stubdomain_override=1" in xl's domain
configuration files) are NOT vulnerable.

Both the traditional ("qemu-xen-traditional") or upstream-based
("qemu-xen") qemu device models are potentially vulnerable.

Systems running only PV guests are NOT vulnerable.

ARM systems are NOT vulnerable.

MITIGATION
==========

Avoiding the use of emulated CD-ROM devices altogether, by not
specifying such devices in the domain configuration, will avoid this
issue.

Enabling stubdomains will mitigate this issue, by reducing the
escalation to only those privileges accorded to the service domain.
qemu-dm stubdomains are only available with "qemu-xen-traditional".

CREDITS
=======

This issue was discovered by Kevin Wolf of Red Hat.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa138-qemut-{1,2}.patch     qemu-xen-traditional, Xen unstable, Xen 4.5.x,
                             Xen 4.4.x, Xen 4.3.x, Xen 4.2.x
xsa138-qemuu-{1,2,3}.patch   qemu-upstream, xen unstable, Xen 4.5.x,
                             Xen 4.4.x, Xen 4.3.x
xsa138-qemuu-{1,3}.patch     qemu-upstream, Xen 4.2.x

NOTE: xsa138-qemuu-2.patch is not required for Xen 4.2.x.

$ sha256sum xsa138*.patch
7e385455379d88658b8ab0d4c1effffe9af21fff2e1dc0fe51cacc779afc83a4  xsa138-qemut-1.patch
c9a89082e36a0646a6fe002c6892d966d415d11ad5cfdcfea7e9c8d7a3f1316c  xsa138-qemut-2.patch
a076808f543c82aeac2f0239a4a46d9baadcd4e4b0a2f9ae7ded99cf59cffde6  xsa138-qemuu-1.patch
ed16dca7d2c179d0931d6e2503264d6593547a803eb3f08f6db7fff2127509a9  xsa138-qemuu-2.patch
090bdec00ede1f0ace1af52833038a74971e060d0c176b42bfca08511d36c644  xsa138-qemuu-3.patch
$

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

Deployment of patches or mitigations is NOT permitted (except on
systems used and administered only by organisations which are members
of the Xen Project Security Issues Predisclosure List).  Specifically,
deployment on public cloud systems is NOT permitted.

The decision not to permit deployment was made by the group that, at
their discretion, disclosed the issue to the Xen Project Security
Team.

Deployment is permitted only AFTER the embargo ends.

(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.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJVth2LAAoJEIP+FMlX6CvZcd4IAJYWZrj86FDn9L5SqeTq8cLX
6tnptNaQb+uDQ/thV2R+nUVdJNaJt1UIhRhO2tD2g0dEqj/I7Vx/Hh95ncPCQ3fS
ec7ph9lcsdAy8E+7abNlhJnPsOVOazEwI0we2deKjdn3CqyfVXqA47rSDY4VChtc
kTV7lEIEebBlo1igz05/poUEhjkCP8UvSfpgpQY60N2y+C0OyIXPIog4q2LiEbeO
cq/deACYN3jOVwPTozkQNAAOq0++UfnGfDredOIYCbvqA5OtMf1DGlWyTQLIEuKJ
zCiatGudJI2klVYkHSVYfXr54WjreiRCOfLB9ilhBW7Yr2juWFQIAc+0Kf09uFo=
=I0Tz
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa138-qemut-1.patch --]
[-- Type: application/octet-stream, Size: 2253 bytes --]

From 510952d4c33ee69574167ce30829b21c815a165b Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 3 Jun 2015 14:13:31 +0200
Subject: [PATCH 1/2] ide: Check array bounds before writing to io_buffer
 (CVE-2015-5154)

If the end_transfer_func of a command is called because enough data has
been read or written for the current PIO transfer, and it fails to
correctly call the command completion functions, the DRQ bit in the
status register and s->end_transfer_func may remain set. This allows the
guest to access further bytes in s->io_buffer beyond s->data_end, and
eventually overflowing the io_buffer.

One case where this currently happens is emulation of the ATAPI command
START STOP UNIT.

This patch fixes the problem by adding explicit array bounds checks
before accessing the buffer instead of relying on end_transfer_func to
function correctly.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/ide.c b/hw/ide.c
index 791666b..211ec88 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -3002,6 +3002,10 @@ static void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
     buffered_pio_write(s, addr, 2);
 
     p = s->data_ptr;
+    if (p + 2 > s->data_end) {
+        return;
+    }
+
     *(uint16_t *)p = le16_to_cpu(val);
     p += 2;
     s->data_ptr = p;
@@ -3021,6 +3025,10 @@ static uint32_t ide_data_readw(void *opaque, uint32_t addr)
     buffered_pio_read(s, addr, 2);
 
     p = s->data_ptr;
+    if (p + 2 > s->data_end) {
+        return 0;
+    }
+
     ret = cpu_to_le16(*(uint16_t *)p);
     p += 2;
     s->data_ptr = p;
@@ -3040,6 +3048,10 @@ static void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
     buffered_pio_write(s, addr, 4);
 
     p = s->data_ptr;
+    if (p + 4 > s->data_end) {
+        return;
+    }
+
     *(uint32_t *)p = le32_to_cpu(val);
     p += 4;
     s->data_ptr = p;
@@ -3059,6 +3071,10 @@ static uint32_t ide_data_readl(void *opaque, uint32_t addr)
     buffered_pio_read(s, addr, 4);
 
     p = s->data_ptr;
+    if (p + 4 > s->data_end) {
+        return 0;
+    }
+
     ret = cpu_to_le32(*(uint32_t *)p);
     p += 4;
     s->data_ptr = p;
-- 
2.1.4


[-- Attachment #3: xsa138-qemut-2.patch --]
[-- Type: application/octet-stream, Size: 2090 bytes --]

From 1ac0f60d558b7fca55c69a61ab4c4538af1f02f9 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 3 Jun 2015 14:41:27 +0200
Subject: [PATCH 2/2] ide: Clear DRQ after handling all expected accesses

This is additional hardening against an end_transfer_func that fails to
clear the DRQ status bit. The bit must be unset as soon as the PIO
transfer has completed, so it's better to do this in a central place
instead of duplicating the code in all commands (and forgetting it in
some).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/ide.c b/hw/ide.c
index 211ec88..7b84d1b 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -3009,8 +3009,10 @@ static void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
     *(uint16_t *)p = le16_to_cpu(val);
     p += 2;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
 }
 
 static uint32_t ide_data_readw(void *opaque, uint32_t addr)
@@ -3032,8 +3034,10 @@ static uint32_t ide_data_readw(void *opaque, uint32_t addr)
     ret = cpu_to_le16(*(uint16_t *)p);
     p += 2;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
     return ret;
 }
 
@@ -3055,8 +3059,10 @@ static void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
     *(uint32_t *)p = le32_to_cpu(val);
     p += 4;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
 }
 
 static uint32_t ide_data_readl(void *opaque, uint32_t addr)
@@ -3078,8 +3084,10 @@ static uint32_t ide_data_readl(void *opaque, uint32_t addr)
     ret = cpu_to_le32(*(uint32_t *)p);
     p += 4;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
     return ret;
 }
 
-- 
2.1.4


[-- Attachment #4: xsa138-qemuu-1.patch --]
[-- Type: application/octet-stream, Size: 2133 bytes --]

From a9de14175548c04e0f8be7fae219246509ba46a9 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 3 Jun 2015 14:13:31 +0200
Subject: [PATCH 1/3] ide: Check array bounds before writing to io_buffer
 (CVE-2015-5154)

If the end_transfer_func of a command is called because enough data has
been read or written for the current PIO transfer, and it fails to
correctly call the command completion functions, the DRQ bit in the
status register and s->end_transfer_func may remain set. This allows the
guest to access further bytes in s->io_buffer beyond s->data_end, and
eventually overflowing the io_buffer.

One case where this currently happens is emulation of the ATAPI command
START STOP UNIT.

This patch fixes the problem by adding explicit array bounds checks
before accessing the buffer instead of relying on end_transfer_func to
function correctly.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 122e955..44fcc23 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2021,6 +2021,10 @@ void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
     }
 
     p = s->data_ptr;
+    if (p + 2 > s->data_end) {
+        return;
+    }
+
     *(uint16_t *)p = le16_to_cpu(val);
     p += 2;
     s->data_ptr = p;
@@ -2042,6 +2046,10 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
     }
 
     p = s->data_ptr;
+    if (p + 2 > s->data_end) {
+        return 0;
+    }
+
     ret = cpu_to_le16(*(uint16_t *)p);
     p += 2;
     s->data_ptr = p;
@@ -2063,6 +2071,10 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
     }
 
     p = s->data_ptr;
+    if (p + 4 > s->data_end) {
+        return;
+    }
+
     *(uint32_t *)p = le32_to_cpu(val);
     p += 4;
     s->data_ptr = p;
@@ -2084,6 +2096,10 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
     }
 
     p = s->data_ptr;
+    if (p + 4 > s->data_end) {
+        return 0;
+    }
+
     ret = cpu_to_le32(*(uint32_t *)p);
     p += 4;
     s->data_ptr = p;
-- 
1.8.3.1

[-- Attachment #5: xsa138-qemuu-2.patch --]
[-- Type: application/octet-stream, Size: 788 bytes --]

From aa851d30acfbb9580098ac1dc82885530cb8b3c1 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 3 Jun 2015 14:17:46 +0200
Subject: [PATCH 2/3] ide/atapi: Fix START STOP UNIT command completion

The command must be completed on all code paths. START STOP UNIT with
pwrcnd set should succeed without doing anything.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/atapi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 950e311..79dd167 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -983,6 +983,7 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)

     if (pwrcnd) {
         /* eject/load only happens for power condition == 0 */
+        ide_atapi_cmd_ok(s);
         return;
     }

--
1.8.3.1


[-- Attachment #6: xsa138-qemuu-3.patch --]
[-- Type: application/octet-stream, Size: 2070 bytes --]

From 1d3c2268f8708126a34064c2e0c1000b40e6f3e5 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 3 Jun 2015 14:41:27 +0200
Subject: [PATCH 3/3] ide: Clear DRQ after handling all expected accesses

This is additional hardening against an end_transfer_func that fails to
clear the DRQ status bit. The bit must be unset as soon as the PIO
transfer has completed, so it's better to do this in a central place
instead of duplicating the code in all commands (and forgetting it in
some).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 44fcc23..50449ca 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2028,8 +2028,10 @@ void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
     *(uint16_t *)p = le16_to_cpu(val);
     p += 2;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
 }

 uint32_t ide_data_readw(void *opaque, uint32_t addr)
@@ -2053,8 +2055,10 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
     ret = cpu_to_le16(*(uint16_t *)p);
     p += 2;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
     return ret;
 }

@@ -2078,8 +2082,10 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
     *(uint32_t *)p = le32_to_cpu(val);
     p += 4;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
 }

 uint32_t ide_data_readl(void *opaque, uint32_t addr)
@@ -2103,8 +2109,10 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
     ret = cpu_to_le32(*(uint32_t *)p);
     p += 4;
     s->data_ptr = p;
-    if (p >= s->data_end)
+    if (p >= s->data_end) {
+        s->status &= ~DRQ_STAT;
         s->end_transfer_func(s);
+    }
     return ret;
 }

--
1.8.3.1


[-- Attachment #7: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2015-07-27 12:03 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-27 12:03 Xen Security Advisory 138 (CVE-2015-5154) - QEMU heap overflow flaw while processing certain ATAPI commands Xen.org security team

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).