[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] Xen Security Advisory 111 (CVE-2014-8866) - Excessive checking in compatibility mode
From: Xen.org security team <security () xen ! org>
Date: 2014-11-27 12:06:23
Message-ID: E1Xtxq3-0008Rw-1V () xenbits ! xen ! org
[Download RAW message or body]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Xen Security Advisory CVE-2014-8866 / XSA-111
version 3
Excessive checking in compatibility mode hypercall argument translation
UPDATES IN VERSION 3
====================
Public release.
ISSUE DESCRIPTION
=================
The hypercall argument translation needed for 32-bit guests running on
64-bit hypervisors performs checks on the final register state. These
checks cover all registers potentially holding hypercall arguments,
not just the ones actually doing so for the hypercall being processed,
since the code was originally intended for use only by PV guests.
While this is not a problem for PV guests (as they can't enter 64-bit
mode and hence can't alter the high halves of any of the registers),
the subsequent reuse of the same functionality for HVM guests exposed
those checks to values (specifically, unexpected values for the high
halves of registers not holding hypercall arguments) controlled by
guest software.
IMPACT
======
A buggy or malicious HVM guest can crash the host.
VULNERABLE SYSTEMS
==================
Xen 3.3 and onward are vulnerable.
Only x86 systems are vulnerable. ARM systems are not vulnerable.
MITIGATION
==========
Running only PV guests will avoid this issue.
There is no mitigation available for HVM guests on any version of Xen
so far released by xenproject.org.
CREDITS
=======
This issue was discovered by Jan Beulich of SUSE.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa111-unstable.patch xen-unstable, Xen 4.4.x
xsa111-4.3.patch Xen 4.3.x
xsa111-4.2.patch Xen 4.2.x
$ sha256sum xsa111*.patch
f6e1bf166ebed6235802e4e42853430d2f5b456c1837908a4f7ed6d4d150e4b4 xsa111-4.2.patch
e9b03a4443a40142cc5c21848dc9589770620dde8924344c4a00028c4dace9f2 xsa111-4.3.patch
3c418f065cd452c225af34c3cccf9bdbc37efb6c6a5fc5940fd83ad8620510d3 xsa111.patch
$
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQEcBAEBAgAGBQJUdwoTAAoJEIP+FMlX6CvZ/jIH/01d45vOe9bUokjixu+sv93n
FPxm2XC9IZEAuDU4h4RXAkzI0L4vuCAnJq0Rr3quizukQ/oqtPPdbYGC/VgQ15LU
0XE3J2U8BbwsweEDIADinJZ76UvvIWtT4/llQT2WCI/g7nRiW7lZAUkhR9nXL2gg
pw48QIdBkgEGZO7JlWEmrA60OwFcAAdG66/IWNjWbUPrscr/DLG0gimrqqAtG9lY
jTpDrOgC+xARbES9iRBt0IU4duMUiCjwy+y8jeq/Ka5d6QIrcaeTO9Y3d6jf2CCE
Z7TC22OGO4XMg6j+abceao3geS29ezsDQttSh7rGjwqMaNqJbIiitKIq4svAtS4=
=Gtqx
-----END PGP SIGNATURE-----
["xsa111-4.2.patch" (application/octet-stream)]
x86: limit checks in hypercall_xlat_continuation() to actual arguments
HVM/PVH guests can otherwise trigger the final BUG_ON() in that
function by entering 64-bit mode, setting the high halves of affected
registers to non-zero values, leaving 64-bit mode, and issuing a
hypercall that might get preempted and hence become subject to
continuation argument translation (HYPERVISOR_memory_op being the only
one possible for HVM, PVH also having the option of using
HYPERVISOR_mmuext_op). This issue got introduced when HVM code was
switched to use compat_memory_op() - neither that nor
hypercall_xlat_continuation() were originally intended to be used by
other than PV guests (which can't enter 64-bit mode and hence have no
way to alter the high halves of 64-bit registers).
This is XSA-111.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1921,7 +1921,8 @@ unsigned long hypercall_create_continuat
}
#ifdef CONFIG_COMPAT
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...)
{
int rc = 0;
struct mc_state *mcs = ¤t->mc_state;
@@ -1930,7 +1931,10 @@ int hypercall_xlat_continuation(unsigned
unsigned long nval = 0;
va_list args;
- BUG_ON(id && *id > 5);
+ ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
+ ASSERT(!(mask >> nr));
+
+ BUG_ON(id && *id >= nr);
BUG_ON(id && (mask & (1U << *id)));
va_start(args, mask);
@@ -1939,7 +1943,7 @@ int hypercall_xlat_continuation(unsigned
{
if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
return 0;
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
if ( mask & 1 )
{
@@ -1967,7 +1971,7 @@ int hypercall_xlat_continuation(unsigned
else
{
regs = guest_cpu_user_regs();
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
unsigned long *reg;
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -78,7 +78,7 @@ int compat_arch_memory_op(int op, XEN_GU
}
if ( rc == __HYPERVISOR_memory_op )
- hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+ hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
break;
}
@@ -144,7 +144,7 @@ int compat_arch_memory_op(int op, XEN_GU
break;
if ( rc == __HYPERVISOR_memory_op )
- hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+ hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
XLAT_pod_target(&cmp, nat);
@@ -379,7 +379,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE(mm
left = 1;
if ( arg1 != MMU_UPDATE_PREEMPTED )
{
- BUG_ON(!hypercall_xlat_continuation(&left, 0x01, nat_ops,
+ BUG_ON(!hypercall_xlat_continuation(&left, 4, 0x01, nat_ops,
cmp_uops));
if ( !test_bit(_MCSF_in_multicall, &mcs->flags) )
regs->_ecx += count - i;
@@ -387,7 +387,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE(mm
mcs->compat_call.args[1] += count - i;
}
else
- BUG_ON(hypercall_xlat_continuation(&left, 0));
+ BUG_ON(hypercall_xlat_continuation(&left, 4, 0));
BUG_ON(left != arg1);
}
else
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -208,7 +208,7 @@ int compat_memory_op(unsigned int cmd, X
break;
cmd = 0;
- if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
+ if ( hypercall_xlat_continuation(&cmd, 2, 0x02, nat.hnd, compat) )
{
BUG_ON(rc != __HYPERVISOR_memory_op);
BUG_ON((cmd & MEMOP_CMD_MASK) != op);
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -192,6 +192,8 @@ static inline int name(k xen_ ## n *x, k
* This option is useful for extracting the "op" argument or similar from the
* hypercall to enable further xlat processing.
*
+ * nr: Total number of arguments the hypercall has.
+ *
* mask: Specifies which of the hypercall arguments require compat translation.
* bit 0 indicates that the 0'th argument requires translation, bit 1 indicates
* that the first argument requires translation and so on. Native and compat
@@ -211,7 +213,8 @@ static inline int name(k xen_ ## n *x, k
*
* Return: Number of arguments which were actually translated.
*/
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...);
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...);
/* In-place translation functons: */
struct start_info;
["xsa111-4.3.patch" (application/octet-stream)]
x86: limit checks in hypercall_xlat_continuation() to actual arguments
HVM/PVH guests can otherwise trigger the final BUG_ON() in that
function by entering 64-bit mode, setting the high halves of affected
registers to non-zero values, leaving 64-bit mode, and issuing a
hypercall that might get preempted and hence become subject to
continuation argument translation (HYPERVISOR_memory_op being the only
one possible for HVM, PVH also having the option of using
HYPERVISOR_mmuext_op). This issue got introduced when HVM code was
switched to use compat_memory_op() - neither that nor
hypercall_xlat_continuation() were originally intended to be used by
other than PV guests (which can't enter 64-bit mode and hence have no
way to alter the high halves of 64-bit registers).
This is XSA-111.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1653,7 +1653,8 @@ unsigned long hypercall_create_continuat
return op;
}
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...)
{
int rc = 0;
struct mc_state *mcs = ¤t->mc_state;
@@ -1662,7 +1663,10 @@ int hypercall_xlat_continuation(unsigned
unsigned long nval = 0;
va_list args;
- BUG_ON(id && *id > 5);
+ ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
+ ASSERT(!(mask >> nr));
+
+ BUG_ON(id && *id >= nr);
BUG_ON(id && (mask & (1U << *id)));
va_start(args, mask);
@@ -1671,7 +1675,7 @@ int hypercall_xlat_continuation(unsigned
{
if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
return 0;
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
if ( mask & 1 )
{
@@ -1699,7 +1703,7 @@ int hypercall_xlat_continuation(unsigned
else
{
regs = guest_cpu_user_regs();
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
unsigned long *reg;
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -78,7 +78,7 @@ int compat_arch_memory_op(int op, XEN_GU
}
if ( rc == __HYPERVISOR_memory_op )
- hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+ hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
break;
}
@@ -144,7 +144,7 @@ int compat_arch_memory_op(int op, XEN_GU
break;
if ( rc == __HYPERVISOR_memory_op )
- hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+ hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
XLAT_pod_target(&cmp, nat);
@@ -379,7 +379,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE_PA
left = 1;
if ( arg1 != MMU_UPDATE_PREEMPTED )
{
- BUG_ON(!hypercall_xlat_continuation(&left, 0x01, nat_ops,
+ BUG_ON(!hypercall_xlat_continuation(&left, 4, 0x01, nat_ops,
cmp_uops));
if ( !test_bit(_MCSF_in_multicall, &mcs->flags) )
regs->_ecx += count - i;
@@ -387,7 +387,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE_PA
mcs->compat_call.args[1] += count - i;
}
else
- BUG_ON(hypercall_xlat_continuation(&left, 0));
+ BUG_ON(hypercall_xlat_continuation(&left, 4, 0));
BUG_ON(left != arg1);
}
else
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -208,7 +208,7 @@ int compat_memory_op(unsigned int cmd, X
break;
cmd = 0;
- if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
+ if ( hypercall_xlat_continuation(&cmd, 2, 0x02, nat.hnd, compat) )
{
BUG_ON(rc != __HYPERVISOR_memory_op);
BUG_ON((cmd & MEMOP_CMD_MASK) != op);
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -195,6 +195,8 @@ static inline int name(k xen_ ## n *x, k
* This option is useful for extracting the "op" argument or similar from the
* hypercall to enable further xlat processing.
*
+ * nr: Total number of arguments the hypercall has.
+ *
* mask: Specifies which of the hypercall arguments require compat translation.
* bit 0 indicates that the 0'th argument requires translation, bit 1 indicates
* that the first argument requires translation and so on. Native and compat
@@ -214,7 +216,8 @@ static inline int name(k xen_ ## n *x, k
*
* Return: Number of arguments which were actually translated.
*/
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...);
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...);
/* In-place translation functons: */
struct start_info;
["xsa111.patch" (application/octet-stream)]
x86: limit checks in hypercall_xlat_continuation() to actual arguments
HVM/PVH guests can otherwise trigger the final BUG_ON() in that
function by entering 64-bit mode, setting the high halves of affected
registers to non-zero values, leaving 64-bit mode, and issuing a
hypercall that might get preempted and hence become subject to
continuation argument translation (HYPERVISOR_memory_op being the only
one possible for HVM, PVH also having the option of using
HYPERVISOR_mmuext_op). This issue got introduced when HVM code was
switched to use compat_memory_op() - neither that nor
hypercall_xlat_continuation() were originally intended to be used by
other than PV guests (which can't enter 64-bit mode and hence have no
way to alter the high halves of 64-bit registers).
This is XSA-111.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1750,7 +1750,8 @@ unsigned long hypercall_create_continuat
return op;
}
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...)
{
int rc = 0;
struct mc_state *mcs = ¤t->mc_state;
@@ -1759,7 +1760,10 @@ int hypercall_xlat_continuation(unsigned
unsigned long nval = 0;
va_list args;
- BUG_ON(id && *id > 5);
+ ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
+ ASSERT(!(mask >> nr));
+
+ BUG_ON(id && *id >= nr);
BUG_ON(id && (mask & (1U << *id)));
va_start(args, mask);
@@ -1772,7 +1776,7 @@ int hypercall_xlat_continuation(unsigned
return 0;
}
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
if ( mask & 1 )
{
@@ -1800,7 +1804,7 @@ int hypercall_xlat_continuation(unsigned
else
{
regs = guest_cpu_user_regs();
- for ( i = 0; i < 6; ++i, mask >>= 1 )
+ for ( i = 0; i < nr; ++i, mask >>= 1 )
{
unsigned long *reg;
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -118,7 +118,7 @@ int compat_arch_memory_op(unsigned long
break;
if ( rc == __HYPERVISOR_memory_op )
- hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+ hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
XLAT_pod_target(&cmp, nat);
@@ -354,7 +354,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE_PA
left = 1;
if ( arg1 != MMU_UPDATE_PREEMPTED )
{
- BUG_ON(!hypercall_xlat_continuation(&left, 0x01, nat_ops,
+ BUG_ON(!hypercall_xlat_continuation(&left, 4, 0x01, nat_ops,
cmp_uops));
if ( !test_bit(_MCSF_in_multicall, &mcs->flags) )
regs->_ecx += count - i;
@@ -362,7 +362,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE_PA
mcs->compat_call.args[1] += count - i;
}
else
- BUG_ON(hypercall_xlat_continuation(&left, 0));
+ BUG_ON(hypercall_xlat_continuation(&left, 4, 0));
BUG_ON(left != arg1);
}
else
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -282,7 +282,7 @@ int compat_memory_op(unsigned int cmd, X
break;
cmd = 0;
- if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
+ if ( hypercall_xlat_continuation(&cmd, 2, 0x02, nat.hnd, compat) )
{
BUG_ON(rc != __HYPERVISOR_memory_op);
BUG_ON((cmd & MEMOP_CMD_MASK) != op);
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -195,6 +195,8 @@ static inline int name(k xen_ ## n *x, k
* This option is useful for extracting the "op" argument or similar from the
* hypercall to enable further xlat processing.
*
+ * nr: Total number of arguments the hypercall has.
+ *
* mask: Specifies which of the hypercall arguments require compat translation.
* bit 0 indicates that the 0'th argument requires translation, bit 1 indicates
* that the first argument requires translation and so on. Native and compat
@@ -214,7 +216,8 @@ static inline int name(k xen_ ## n *x, k
*
* Return: Number of arguments which were actually translated.
*/
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...);
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+ unsigned int mask, ...);
/* In-place translation functons: */
struct start_info;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic