[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 112 (CVE-2014-8867) - Insufficient bounding of "REP MOVS" to MM
From:       Xen.org security team <security () xen ! org>
Date:       2014-11-27 12:08:53
Message-ID: E1XtxsT-0000TV-4i () xenbits ! xen ! org
[Download RAW message or body]

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

            Xen Security Advisory CVE-2014-8867 / XSA-112
                              version 5

  Insufficient bounding of "REP MOVS" to MMIO emulated inside the hypervisor

UPDATES IN VERSION 5
====================

Public release.

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

Acceleration support for the "REP MOVS" instruction, when the first
iteration accesses memory mapped I/O emulated internally in the
hypervisor, incorrectly assumes that the whole range accessed is
handled by the same hypervisor sub-component.

IMPACT
======

A buggy or malicious HVM guest can crash the host.

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

Xen versions from at least 3.2.x onwards are vulnerable on x86 systems.
Older versions have not been inspected.  ARM systems are not vulnerable.

MITIGATION
==========

Running only PV guests will avoid this issue.

There is no mitigation available for HVM guests.

CREDITS
=======

This issue was discovered by Jan Beulich of SUSE.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa112-unstable.patch        xen-unstable, Xen 4.4.x, Xen 4.3.x
xsa112-4.2.patch             Xen 4.2.x

$ sha256sum xsa112*.patch
cf01a1acd258e7cbb3586e543ba3668c1ee7fb05cba19b8b5369a3e101a2288f  xsa112-4.2.patch
cc39a4cdcb52929ed36ab696807d2405aa552177a6f029d8a1a52041ca1ed519  xsa112.patch
$

We have been told that this patch is not sufficient on Xen 3.3.x and
earlier without also backporting b1b6362f (git commit id).

Note that while we are happy to share information we receive about
earlier Xen versions, the earliest Xen branch for which the Xen
Project offers security support is 4.2.x.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJUdwoNAAoJEIP+FMlX6CvZfekIAMBq3ynRyuyqvukMhSBaFj2O
SBX747HJPKRmoODGZGe9EJ0pAJhckQ00RaKFulxSLzFeu4Oi6M3GrvNCvST0sR54
bLTmeNeBOhLef4ylDqAWOSY4C7AJW/jC1ngtSy3wd6zuwFD0bzPYb7nk94PD32ie
9LYTt+FSkoo/3j3IviCqNVXTlMmhmdjP0U3+xXgxQZ9y47zTT8gsX4KoplC/i1Wq
uhla/ZYI+Ro/ejYVHsKDDhfA1mgAGDoOLhmNEBLHPzTyGs4VOSaXzX7wce8JWpBi
oXdnN5HW80mmkZ6qI42/bnvpSHTqm+QVFD0v1Uz0cSrBYJGq6LULBAmaJHGldDA=
=8eF1
-----END PGP SIGNATURE-----

["xsa112-4.2.patch" (application/octet-stream)]

x86/HVM: confine internally handled MMIO to solitary regions

While it is generally wrong to cross region boundaries when dealing
with MMIO accesses of repeated string instructions (currently only
MOVS) as that would do things a guest doesn't expect (leaving aside
that none of these regions would normally be accessed with repeated
string instructions in the first place), this is even more of a problem
for all virtual MSI-X page accesses (both msixtbl_{read,write}() can be
made dereference NULL "entry" pointers this way) as well as undersized
(1- or 2-byte) LAPIC writes (causing vlapic_read_aligned() to access
space beyond the one memory page set up for holding LAPIC register
values).

Since those functions validly assume to be called only with addresses
their respective checking functions indicated to be okay, it is generic
code that needs to be fixed to clip the repetition count.

To be on the safe side (and consistent), also do the same for buffered
I/O intercepts, even if their only client (stdvga) doesn't put the
hypervisor at risk (i.e. "only" guest misbehavior would result).

This is CVE-2014-8867 / XSA-112.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>

--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -131,11 +131,24 @@ int hvm_mmio_intercept(ioreq_t *p)
     int i;
 
     for ( i = 0; i < HVM_MMIO_HANDLER_NR; i++ )
-        if ( hvm_mmio_handlers[i]->check_handler(v, p->addr) )
+    {
+        hvm_mmio_check_t check_handler =
+            hvm_mmio_handlers[i]->check_handler;
+
+        if ( check_handler(v, p->addr) )
+        {
+            if ( unlikely(p->count > 1) &&
+                 !check_handler(v, unlikely(p->df)
+                                   ? p->addr - (p->count - 1LL) * p->size
+                                   : p->addr + (p->count - 1LL) * p->size) )
+                p->count = 1;
+
             return hvm_mmio_access(
                 v, p,
                 hvm_mmio_handlers[i]->read_handler,
                 hvm_mmio_handlers[i]->write_handler);
+        }
+    }
 
     return X86EMUL_UNHANDLEABLE;
 }
@@ -243,6 +256,13 @@ int hvm_io_intercept(ioreq_t *p, int typ
             if ( type == HVM_PORTIO )
                 return process_portio_intercept(
                     handler->hdl_list[i].action.portio, p);
+
+            if ( unlikely(p->count > 1) &&
+                 (unlikely(p->df)
+                  ? p->addr - (p->count - 1LL) * p->size < addr
+                  : p->addr + p->count * 1LL * p->size - 1 >= addr + size) )
+                p->count = 1;
+
             return handler->hdl_list[i].action.mmio(p);
         }
     }
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -236,6 +236,8 @@ static int msixtbl_read(
     rcu_read_lock(&msixtbl_rcu_lock);
 
     entry = msixtbl_find_entry(v, address);
+    if ( !entry )
+        goto out;
     offset = address & (PCI_MSIX_ENTRY_SIZE - 1);
 
     if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET )
@@ -278,6 +280,8 @@ static int msixtbl_write(struct vcpu *v,
     rcu_read_lock(&msixtbl_rcu_lock);
 
     entry = msixtbl_find_entry(v, address);
+    if ( !entry )
+        goto out;
     nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE;
 
     offset = address & (PCI_MSIX_ENTRY_SIZE - 1);

["xsa112.patch" (application/octet-stream)]

x86/HVM: confine internally handled MMIO to solitary regions

While it is generally wrong to cross region boundaries when dealing
with MMIO accesses of repeated string instructions (currently only
MOVS) as that would do things a guest doesn't expect (leaving aside
that none of these regions would normally be accessed with repeated
string instructions in the first place), this is even more of a problem
for all virtual MSI-X page accesses (both msixtbl_{read,write}() can be
made dereference NULL "entry" pointers this way) as well as undersized
(1- or 2-byte) LAPIC writes (causing vlapic_read_aligned() to access
space beyond the one memory page set up for holding LAPIC register
values).

Since those functions validly assume to be called only with addresses
their respective checking functions indicated to be okay, it is generic
code that needs to be fixed to clip the repetition count.

To be on the safe side (and consistent), also do the same for buffered
I/O intercepts, even if their only client (stdvga) doesn't put the
hypervisor at risk (i.e. "only" guest misbehavior would result).

This is CVE-2014-8867 / XSA-112.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>

--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -181,11 +181,24 @@ int hvm_mmio_intercept(ioreq_t *p)
     int i;
 
     for ( i = 0; i < HVM_MMIO_HANDLER_NR; i++ )
-        if ( hvm_mmio_handlers[i]->check_handler(v, p->addr) )
+    {
+        hvm_mmio_check_t check_handler =
+            hvm_mmio_handlers[i]->check_handler;
+
+        if ( check_handler(v, p->addr) )
+        {
+            if ( unlikely(p->count > 1) &&
+                 !check_handler(v, unlikely(p->df)
+                                   ? p->addr - (p->count - 1L) * p->size
+                                   : p->addr + (p->count - 1L) * p->size) )
+                p->count = 1;
+
             return hvm_mmio_access(
                 v, p,
                 hvm_mmio_handlers[i]->read_handler,
                 hvm_mmio_handlers[i]->write_handler);
+        }
+    }
 
     return X86EMUL_UNHANDLEABLE;
 }
@@ -342,6 +355,13 @@ int hvm_io_intercept(ioreq_t *p, int typ
             if ( type == HVM_PORTIO )
                 return process_portio_intercept(
                     handler->hdl_list[i].action.portio, p);
+
+            if ( unlikely(p->count > 1) &&
+                 (unlikely(p->df)
+                  ? p->addr - (p->count - 1L) * p->size < addr
+                  : p->addr + p->count * 1L * p->size - 1 >= addr + size) )
+                p->count = 1;
+
             return handler->hdl_list[i].action.mmio(p);
         }
     }
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -226,6 +226,8 @@ static int msixtbl_read(
     rcu_read_lock(&msixtbl_rcu_lock);
 
     entry = msixtbl_find_entry(v, address);
+    if ( !entry )
+        goto out;
     offset = address & (PCI_MSIX_ENTRY_SIZE - 1);
 
     if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET )
@@ -268,6 +270,8 @@ static int msixtbl_write(struct vcpu *v,
     rcu_read_lock(&msixtbl_rcu_lock);
 
     entry = msixtbl_find_entry(v, address);
+    if ( !entry )
+        goto out;
     nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE;
 
     offset = address & (PCI_MSIX_ENTRY_SIZE - 1);


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic