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

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 63 (CVE-2013-4355) - Information leaks through I/O instruction
From:       Xen.org security team <security () xen ! org>
Date:       2013-09-30 12:04:15
Message-ID: E1VQcD1-0000Nq-BD () xenbits ! xen ! org
[Download RAW message or body]

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

              Xen Security Advisory CVE-2013-4355 / XSA-63
                             version 3

         Information leaks through I/O instruction emulation

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

Public release.

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

Insufficient or missing error handling in certain routines dealing
with guest memory reads can lead to uninitialized data on the
hypervisor stack (potentially containing sensitive data from prior
work the hypervisor performed) being copied to guest visible storage.

This allows a malicious HVM guest to craft certain operations (namely,
but not limited to, port or memory mapped I/O writes) involving
physical or virtual addresses that have no actual memory associated
with them, so that hypervisor stack contents are copied into the
destination of the operation, thus becoming visible to the guest.

IMPACT
======

A malicious HVM guest might be able to read sensitive data relating
to other guests.

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

Xen 3.2.x and later are vulnerable.
Xen 3.1.x and earlier have not been inspected.

Only HVM guests can take advantage of this vulnerability.

MITIGATION
==========

Running only PV guests will avoid this issue.

CREDITS
=======

This issue was discovered by Coverity Scan and diagnosed by Andrew
Cooper & Tim Deegan.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa63.patch        Xen 4.2.x, 4.3.x, and unstable

$ sha256sum xsa63*.patch
32fa93d8ebdfbe85931c52010bf9e561fdae8846462c5b1f2fbc217ca36f3005  xsa63.patch
$
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJSSUhEAAoJEIP+FMlX6CvZGUsH/13jBs/EU8H/mqXCO7gQXIrm
tPp/gsjxxxhVrwOjmmJZShQ8CWU8T3zL0RKaaGBJzAd+imnXQdb+il1vkNYT8edH
zSB9WN3o/WNu7bzlhm3ro67WlwhXSY2yea7Bj/9bg2//T5RgoXsewX+LbCAJ3Z44
fflCQsCuvpl77oIcftIe5rcJAtHR4Jb5/4Ps+MzxI52oS3m2BGXv/qOTpDfy7qsp
7j/219hChnGVoZ1u/2m0i1789/9tYWM7jFbvqVYH6yHTEgk1ds8Cnn/uHQ8zXjKI
CW8E5HGKOHOpTtJjDF0h3OqcK8vG7qKgHULDziXV//QWPP3uH/dAQCjQO9uS8r4=
=RilU
-----END PGP SIGNATURE-----

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

x86: properly handle hvm_copy_from_guest_{phys,virt}() errors

Ignoring them generally implies using uninitialized data and, in all
cases dealt with here, potentially leaking hypervisor stack contents to
guests.

This is XSA-63.

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

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2308,11 +2308,7 @@ void hvm_task_switch(
 
     rc = hvm_copy_from_guest_virt(
         &tss, prev_tr.base, sizeof(tss), PFEC_page_present);
-    if ( rc == HVMCOPY_bad_gva_to_gfn )
-        goto out;
-    if ( rc == HVMCOPY_gfn_paged_out )
-        goto out;
-    if ( rc == HVMCOPY_gfn_shared )
+    if ( rc != HVMCOPY_okay )
         goto out;
 
     eflags = regs->eflags;
@@ -2357,13 +2353,11 @@ void hvm_task_switch(
 
     rc = hvm_copy_from_guest_virt(
         &tss, tr.base, sizeof(tss), PFEC_page_present);
-    if ( rc == HVMCOPY_bad_gva_to_gfn )
-        goto out;
-    if ( rc == HVMCOPY_gfn_paged_out )
-        goto out;
-    /* Note: this could be optimised, if the callee functions knew we want RO
-     * access */
-    if ( rc == HVMCOPY_gfn_shared )
+    /*
+     * Note: The HVMCOPY_gfn_shared case could be optimised, if the callee
+     * functions knew we want RO access.
+     */
+    if ( rc != HVMCOPY_okay )
         goto out;
 
 
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -87,17 +87,28 @@ static int hvm_mmio_access(struct vcpu *
     {
         for ( i = 0; i < p->count; i++ )
         {
-            int ret;
-
-            ret = hvm_copy_from_guest_phys(&data,
-                                           p->data + (sign * i * p->size),
-                                           p->size);
-            if ( (ret == HVMCOPY_gfn_paged_out) || 
-                 (ret == HVMCOPY_gfn_shared) )
+            switch ( hvm_copy_from_guest_phys(&data,
+                                              p->data + sign * i * p->size,
+                                              p->size) )
             {
+            case HVMCOPY_okay:
+                break;
+            case HVMCOPY_gfn_paged_out:
+            case HVMCOPY_gfn_shared:
                 rc = X86EMUL_RETRY;
                 break;
+            case HVMCOPY_bad_gfn_to_mfn:
+                data = ~0;
+                break;
+            case HVMCOPY_bad_gva_to_gfn:
+                ASSERT(0);
+                /* fall through */
+            default:
+                rc = X86EMUL_UNHANDLEABLE;
+                break;
             }
+            if ( rc != X86EMUL_OKAY )
+                break;
             rc = write_handler(v, p->addr + (sign * i * p->size), p->size,
                                data);
             if ( rc != X86EMUL_OKAY )
@@ -165,8 +176,28 @@ static int process_portio_intercept(port
         for ( i = 0; i < p->count; i++ )
         {
             data = 0;
-            (void)hvm_copy_from_guest_phys(&data, p->data + sign*i*p->size,
-                                           p->size);
+            switch ( hvm_copy_from_guest_phys(&data,
+                                              p->data + sign * i * p->size,
+                                              p->size) )
+            {
+            case HVMCOPY_okay:
+                break;
+            case HVMCOPY_gfn_paged_out:
+            case HVMCOPY_gfn_shared:
+                rc = X86EMUL_RETRY;
+                break;
+            case HVMCOPY_bad_gfn_to_mfn:
+                data = ~0;
+                break;
+            case HVMCOPY_bad_gva_to_gfn:
+                ASSERT(0);
+                /* fall through */
+            default:
+                rc = X86EMUL_UNHANDLEABLE;
+                break;
+            }
+            if ( rc != X86EMUL_OKAY )
+                break;
             rc = action(IOREQ_WRITE, p->addr, p->size, &data);
             if ( rc != X86EMUL_OKAY )
                 break;
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -340,14 +340,24 @@ static int dpci_ioport_write(uint32_t mp
         data = p->data;
         if ( p->data_is_ptr )
         {
-            int ret;
-            
-            ret = hvm_copy_from_guest_phys(&data, 
-                                           p->data + (sign * i * p->size),
-                                           p->size);
-            if ( (ret == HVMCOPY_gfn_paged_out) &&
-                 (ret == HVMCOPY_gfn_shared) )
+            switch ( hvm_copy_from_guest_phys(&data,
+                                              p->data + sign * i * p->size,
+                                              p->size) )
+            {
+            case HVMCOPY_okay:
+                break;
+            case HVMCOPY_gfn_paged_out:
+            case HVMCOPY_gfn_shared:
                 return X86EMUL_RETRY;
+            case HVMCOPY_bad_gfn_to_mfn:
+                data = ~0;
+                break;
+            case HVMCOPY_bad_gva_to_gfn:
+                ASSERT(0);
+                /* fall through */
+            default:
+                return X86EMUL_UNHANDLEABLE;
+            }
         }
 
         switch ( p->size )
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -39,7 +39,9 @@ static void realmode_deliver_exception(
 
  again:
     last_byte = (vector * 4) + 3;
-    if ( idtr->limit < last_byte )
+    if ( idtr->limit < last_byte ||
+         hvm_copy_from_guest_phys(&cs_eip, idtr->base + vector * 4, 4) !=
+         HVMCOPY_okay )
     {
         /* Software interrupt? */
         if ( insn_len != 0 )
@@ -64,8 +66,6 @@ static void realmode_deliver_exception(
         }
     }
 
-    (void)hvm_copy_from_guest_phys(&cs_eip, idtr->base + vector * 4, 4);
-
     frame[0] = regs->eip + insn_len;
     frame[1] = csr->sel;
     frame[2] = regs->eflags & ~X86_EFLAGS_RF;


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

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