All of lore.kernel.org
 help / color / mirror / Atom feed
* Xen Security Advisory 213 - x86: 64bit PV guest breakout via pagetable use-after-mode-change
@ 2017-05-02 12:00 Xen.org security team
  0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2017-05-02 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: 4213 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

                    Xen Security Advisory XSA-213
                              version 2

   x86: 64bit PV guest breakout via pagetable use-after-mode-change

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

Public release.

Added email header syntax to patches, for e.g. git-am.

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

64-bit PV guests typically use separate (root) page tables for their
kernel and user modes.  Hypercalls are accessible to guest kernel
context only, which certain hypercall handlers make assumptions on.
The IRET hypercall (replacing the identically name CPU instruction)
is used by guest kernels to transfer control from kernel mode to user
mode.  If such an IRET hypercall is placed in the middle of a multicall
batch, subsequent operations invoked by the same multicall batch may
wrongly assume the guest to still be in kernel mode.  If one or more of
these subsequent operations involve operations on page tables, they may
be using the wrong root page table, confusing internal accounting.  As
a result the guest may gain writable access to some of its page tables.

IMPACT
======

A malicious or buggy 64-bit PV guest may be able to access all of
system memory, allowing for all of privilege escalation, host crashes,
and information leaks.

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

All 64-bit Xen versions are vulnerable.

Only x86 systems are affected.  ARM systems are not vulnerable.

The vulnerability is only exposed to 64-bit PV guests.  HVM guests and
32-bit PV guests can't exploit the vulnerability.

MITIGATION
==========

Running only HVM or 32-bit PV guests will avoid the vulnerability.

The vulnerability can be avoided if the guest kernel is controlled by
the host rather than guest administrator, provided that further steps
are taken to prevent the guest administrator from loading code into
the kernel (e.g. by disabling loadable modules etc) or from using
other mechanisms which allow them to run code at kernel privilege.

CREDITS
=======

This issue was discovered by Jann Horn of Google Project Zero.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa213.patch           xen-unstable
xsa213-4.8.patch       Xen 4.8.x
xsa213-4.7.patch       Xen 4.7.x
xsa213-4.6.patch       Xen 4.6.x
xsa213-4.5.patch       Xen 4.5.x

$ sha256sum xsa213*
cddea5eac2ad1f5a68b561da4e98afce891189a2fdedf93087a03889e9df6e99  xsa213.patch
fce9bbc9fc30769dfbab4d1830d87d220000b2742e5e70aac22f3e9d013b7614  xsa213-4.5.patch
dce026ed1a02db1cf22de89120e7129839f656d041379c450e7403ae909e7b99  xsa213-4.6.patch
d8202db5981e2f13d9942332cd3fefded98a5cbc302caee431c7a15051887e7f  xsa213-4.7.patch
20c12810ac73809ba74cfde811d420b1b544a07f759c393380afde1a09eb5274  xsa213-4.8.patch
$

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

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

But: 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

iQEcBAEBCAAGBQJZCGr/AAoJEIP+FMlX6CvZ+s8IALroAx1MO5vn6Z0LY2noH+B3
LP32EfS6jzA210jXT1txfjEIFta7In03nCv3KQZZmvFWjIiDTBD/N8THg9XQHC0r
R+FC0yTFXnLNluBY5FqOXf7C7pd3+N+onAMsRIJkaJiDMIL+xtfnLOTFpr9FrVSy
pemRRr1vZuekeph7G446R04lXBCn5pRMj/v1abXjhAFq1leW9hI3vZII/oRpPUCF
BCJysglvQEgk7Qh3Iqhi8nuqAj+IHxGD3udhsruwruzQ+u2XCLA5FeYo0GK+e9AF
aSf+GL9lZIfVj+2v754Gh6xXSe2K/+Ok/8S5FRJQrGD+vQL+UUGT7GTfJEAPvYg=
=meSL
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa213.patch --]
[-- Type: application/octet-stream, Size: 5626 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: multicall: deal with early exit conditions

In particular changes to guest privilege level require the multicall
sequence to be aborted, as hypercalls are permitted from kernel mode
only. While likely not very useful in a multicall, also properly handle
the return value in the HYPERVISOR_iret case (which should be the guest
specified value).

This is XSA-213.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1653,7 +1653,7 @@ static bool check_multicall_32bit_clean(
     return true;
 }
 
-void arch_do_multicall_call(struct mc_state *state)
+enum mc_disposition arch_do_multicall_call(struct mc_state *state)
 {
     struct multicall_entry *multi = &state->call;
     arm_hypercall_fn_t call = NULL;
@@ -1661,23 +1661,26 @@ void arch_do_multicall_call(struct mc_st
     if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     call = arm_hypercall_table[multi->op].fn;
     if ( call == NULL )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     if ( is_32bit_domain(current->domain) &&
          !check_multicall_32bit_clean(multi) )
-        return;
+        return mc_continue;
 
     multi->result = call(multi->args[0], multi->args[1],
                          multi->args[2], multi->args[3],
                          multi->args[4]);
+
+    return likely(!psr_mode_is_user(guest_cpu_user_regs()))
+           ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -215,15 +215,19 @@ void pv_hypercall(struct cpu_user_regs *
     perfc_incr(hypercalls);
 }
 
-void arch_do_multicall_call(struct mc_state *state)
+enum mc_disposition arch_do_multicall_call(struct mc_state *state)
 {
-    if ( !is_pv_32bit_vcpu(current) )
+    struct vcpu *curr = current;
+    unsigned long op;
+
+    if ( !is_pv_32bit_vcpu(curr) )
     {
         struct multicall_entry *call = &state->call;
 
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].native )
-            call->result = pv_hypercall_table[call->op].native(
+        op = call->op;
+        if ( (op < ARRAY_SIZE(pv_hypercall_table)) &&
+             pv_hypercall_table[op].native )
+            call->result = pv_hypercall_table[op].native(
                 call->args[0], call->args[1], call->args[2],
                 call->args[3], call->args[4], call->args[5]);
         else
@@ -234,15 +238,21 @@ void arch_do_multicall_call(struct mc_st
     {
         struct compat_multicall_entry *call = &state->compat_call;
 
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].compat )
-            call->result = pv_hypercall_table[call->op].compat(
+        op = call->op;
+        if ( (op < ARRAY_SIZE(pv_hypercall_table)) &&
+             pv_hypercall_table[op].compat )
+            call->result = pv_hypercall_table[op].compat(
                 call->args[0], call->args[1], call->args[2],
                 call->args[3], call->args[4], call->args[5]);
         else
             call->result = -ENOSYS;
     }
 #endif
+
+    return unlikely(op == __HYPERVISOR_iret)
+           ? mc_exit
+           : likely(guest_kernel_mode(curr, guest_cpu_user_regs()))
+             ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -39,6 +39,7 @@ do_multicall(
     struct mc_state *mcs = &current->mc_state;
     uint32_t         i;
     int              rc = 0;
+    enum mc_disposition disp = mc_continue;
 
     if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
     {
@@ -49,7 +50,7 @@ do_multicall(
     if ( unlikely(!guest_handle_okay(call_list, nr_calls)) )
         rc = -EFAULT;
 
-    for ( i = 0; !rc && i < nr_calls; i++ )
+    for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ )
     {
         if ( i && hypercall_preempt_check() )
             goto preempted;
@@ -62,7 +63,7 @@ do_multicall(
 
         trace_multicall_call(&mcs->call);
 
-        arch_do_multicall_call(mcs);
+        disp = arch_do_multicall_call(mcs);
 
 #ifndef NDEBUG
         {
@@ -76,7 +77,14 @@ do_multicall(
         }
 #endif
 
-        if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
+        if ( unlikely(disp == mc_exit) )
+        {
+            if ( __copy_field_to_guest(call_list, &mcs->call, result) )
+                /* nothing, best effort only */;
+            rc = mcs->call.result;
+        }
+        else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call,
+                                                 result)) )
             rc = -EFAULT;
         else if ( current->hcall_preempted )
         {
@@ -94,6 +102,9 @@ do_multicall(
             guest_handle_add_offset(call_list, 1);
     }
 
+    if ( unlikely(disp == mc_preempt) && i < nr_calls )
+        goto preempted;
+
     perfc_incr(calls_to_multicall);
     perfc_add(calls_from_multicall, i);
     mcs->flags = 0;
--- a/xen/include/xen/multicall.h
+++ b/xen/include/xen/multicall.h
@@ -22,6 +22,10 @@ struct mc_state {
     };
 };
 
-void arch_do_multicall_call(struct mc_state *mc);
+enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+} arch_do_multicall_call(struct mc_state *mc);
 
 #endif /* __XEN_MULTICALL_H__ */

[-- Attachment #3: xsa213-4.5.patch --]
[-- Type: application/octet-stream, Size: 5760 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: multicall: deal with early exit conditions

In particular changes to guest privilege level require the multicall
sequence to be aborted, as hypercalls are permitted from kernel mode
only. While likely not very useful in a multicall, also properly handle
the return value in the HYPERVISOR_iret case (which should be the guest
specified value).

This is XSA-213.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1395,30 +1395,33 @@ static bool_t check_multicall_32bit_clea
     return true;
 }
 
-void do_multicall_call(struct multicall_entry *multi)
+enum mc_disposition do_multicall_call(struct multicall_entry *multi)
 {
     arm_hypercall_fn_t call = NULL;
 
     if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     call = arm_hypercall_table[multi->op].fn;
     if ( call == NULL )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     if ( is_32bit_domain(current->domain) &&
          !check_multicall_32bit_clean(multi) )
-        return;
+        return mc_continue;
 
     multi->result = call(multi->args[0], multi->args[1],
                          multi->args[2], multi->args[3],
                          multi->args[4]);
+
+    return likely(!psr_mode_is_user(guest_cpu_user_regs()))
+           ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -40,6 +40,7 @@ do_multicall(
     struct mc_state *mcs = &current->mc_state;
     uint32_t         i;
     int              rc = 0;
+    enum mc_disposition disp = mc_continue;
 
     if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
     {
@@ -50,7 +51,7 @@ do_multicall(
     if ( unlikely(!guest_handle_okay(call_list, nr_calls)) )
         rc = -EFAULT;
 
-    for ( i = 0; !rc && i < nr_calls; i++ )
+    for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ )
     {
         if ( i && hypercall_preempt_check() )
             goto preempted;
@@ -63,7 +64,7 @@ do_multicall(
 
         trace_multicall_call(&mcs->call);
 
-        do_multicall_call(&mcs->call);
+        disp = do_multicall_call(&mcs->call);
 
 #ifndef NDEBUG
         {
@@ -77,7 +78,14 @@ do_multicall(
         }
 #endif
 
-        if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
+        if ( unlikely(disp == mc_exit) )
+        {
+            if ( __copy_field_to_guest(call_list, &mcs->call, result) )
+                /* nothing, best effort only */;
+            rc = mcs->call.result;
+        }
+        else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call,
+                                                 result)) )
             rc = -EFAULT;
         else if ( test_bit(_MCSF_call_preempted, &mcs->flags) )
         {
@@ -93,6 +101,9 @@ do_multicall(
             guest_handle_add_offset(call_list, 1);
     }
 
+    if ( unlikely(disp == mc_preempt) && i < nr_calls )
+        goto preempted;
+
     perfc_incr(calls_to_multicall);
     perfc_add(calls_from_multicall, i);
     mcs->flags = 0;
--- a/xen/include/asm-arm/multicall.h
+++ b/xen/include/asm-arm/multicall.h
@@ -1,7 +1,11 @@
 #ifndef __ASM_ARM_MULTICALL_H__
 #define __ASM_ARM_MULTICALL_H__
 
-extern void do_multicall_call(struct multicall_entry *call);
+extern enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+} do_multicall_call(struct multicall_entry *call);
 
 #endif /* __ASM_ARM_MULTICALL_H__ */
 /*
--- a/xen/include/asm-x86/multicall.h
+++ b/xen/include/asm-x86/multicall.h
@@ -7,8 +7,21 @@
 
 #include <xen/errno.h>
 
+enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+};
+
+#define multicall_ret(call)                                  \
+    (unlikely((call)->op == __HYPERVISOR_iret)               \
+     ? mc_exit                                               \
+       : likely(guest_kernel_mode(current,                   \
+                                  guest_cpu_user_regs()))    \
+         ? mc_continue : mc_preempt)
+
 #define do_multicall_call(_call)                             \
-    do {                                                     \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movq  %c1(%0),%%rax; "                      \
             "    leaq  hypercall_table(%%rip),%%rdi; "       \
@@ -36,9 +49,11 @@
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
               "r8",  "r9",  "r10", "r11" );                  \
-    } while ( 0 )
+        multicall_ret(_call);                                \
+    })
 
 #define compat_multicall_call(_call)                         \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movl  %c1(%0),%%eax; "                      \
             "    leaq  compat_hypercall_table(%%rip),%%rdi; "\
@@ -65,6 +80,8 @@
               "i" (offsetof(__typeof__(*_call), result))     \
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
-              "r8",  "r9",  "r10", "r11" )                   \
+              "r8",  "r9",  "r10", "r11" );                  \
+        multicall_ret(_call);                                \
+    })
 
 #endif /* __ASM_X86_MULTICALL_H__ */

[-- Attachment #4: xsa213-4.6.patch --]
[-- Type: application/octet-stream, Size: 5760 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: multicall: deal with early exit conditions

In particular changes to guest privilege level require the multicall
sequence to be aborted, as hypercalls are permitted from kernel mode
only. While likely not very useful in a multicall, also properly handle
the return value in the HYPERVISOR_iret case (which should be the guest
specified value).

This is XSA-213.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1485,30 +1485,33 @@ static bool_t check_multicall_32bit_clea
     return true;
 }
 
-void do_multicall_call(struct multicall_entry *multi)
+enum mc_disposition do_multicall_call(struct multicall_entry *multi)
 {
     arm_hypercall_fn_t call = NULL;
 
     if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     call = arm_hypercall_table[multi->op].fn;
     if ( call == NULL )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     if ( is_32bit_domain(current->domain) &&
          !check_multicall_32bit_clean(multi) )
-        return;
+        return mc_continue;
 
     multi->result = call(multi->args[0], multi->args[1],
                          multi->args[2], multi->args[3],
                          multi->args[4]);
+
+    return likely(!psr_mode_is_user(guest_cpu_user_regs()))
+           ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -40,6 +40,7 @@ do_multicall(
     struct mc_state *mcs = &current->mc_state;
     uint32_t         i;
     int              rc = 0;
+    enum mc_disposition disp = mc_continue;
 
     if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
     {
@@ -50,7 +51,7 @@ do_multicall(
     if ( unlikely(!guest_handle_okay(call_list, nr_calls)) )
         rc = -EFAULT;
 
-    for ( i = 0; !rc && i < nr_calls; i++ )
+    for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ )
     {
         if ( i && hypercall_preempt_check() )
             goto preempted;
@@ -63,7 +64,7 @@ do_multicall(
 
         trace_multicall_call(&mcs->call);
 
-        do_multicall_call(&mcs->call);
+        disp = do_multicall_call(&mcs->call);
 
 #ifndef NDEBUG
         {
@@ -77,7 +78,14 @@ do_multicall(
         }
 #endif
 
-        if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
+        if ( unlikely(disp == mc_exit) )
+        {
+            if ( __copy_field_to_guest(call_list, &mcs->call, result) )
+                /* nothing, best effort only */;
+            rc = mcs->call.result;
+        }
+        else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call,
+                                                 result)) )
             rc = -EFAULT;
         else if ( test_bit(_MCSF_call_preempted, &mcs->flags) )
         {
@@ -93,6 +101,9 @@ do_multicall(
             guest_handle_add_offset(call_list, 1);
     }
 
+    if ( unlikely(disp == mc_preempt) && i < nr_calls )
+        goto preempted;
+
     perfc_incr(calls_to_multicall);
     perfc_add(calls_from_multicall, i);
     mcs->flags = 0;
--- a/xen/include/asm-arm/multicall.h
+++ b/xen/include/asm-arm/multicall.h
@@ -1,7 +1,11 @@
 #ifndef __ASM_ARM_MULTICALL_H__
 #define __ASM_ARM_MULTICALL_H__
 
-extern void do_multicall_call(struct multicall_entry *call);
+extern enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+} do_multicall_call(struct multicall_entry *call);
 
 #endif /* __ASM_ARM_MULTICALL_H__ */
 /*
--- a/xen/include/asm-x86/multicall.h
+++ b/xen/include/asm-x86/multicall.h
@@ -7,8 +7,21 @@
 
 #include <xen/errno.h>
 
+enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+};
+
+#define multicall_ret(call)                                  \
+    (unlikely((call)->op == __HYPERVISOR_iret)               \
+     ? mc_exit                                               \
+       : likely(guest_kernel_mode(current,                   \
+                                  guest_cpu_user_regs()))    \
+         ? mc_continue : mc_preempt)
+
 #define do_multicall_call(_call)                             \
-    do {                                                     \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movq  %c1(%0),%%rax; "                      \
             "    leaq  hypercall_table(%%rip),%%rdi; "       \
@@ -37,9 +50,11 @@
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
               "r8",  "r9",  "r10", "r11" );                  \
-    } while ( 0 )
+        multicall_ret(_call);                                \
+    })
 
 #define compat_multicall_call(_call)                         \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movl  %c1(%0),%%eax; "                      \
             "    leaq  compat_hypercall_table(%%rip),%%rdi; "\
@@ -67,6 +82,8 @@
               "i" (-ENOSYS)                                  \
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
-              "r8",  "r9",  "r10", "r11" )                   \
+              "r8",  "r9",  "r10", "r11" );                  \
+        multicall_ret(_call);                                \
+    })
 
 #endif /* __ASM_X86_MULTICALL_H__ */

[-- Attachment #5: xsa213-4.7.patch --]
[-- Type: application/octet-stream, Size: 5749 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: multicall: deal with early exit conditions

In particular changes to guest privilege level require the multicall
sequence to be aborted, as hypercalls are permitted from kernel mode
only. While likely not very useful in a multicall, also properly handle
the return value in the HYPERVISOR_iret case (which should be the guest
specified value).

This is XSA-213.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1529,30 +1529,33 @@ static bool_t check_multicall_32bit_clea
     return true;
 }
 
-void do_multicall_call(struct multicall_entry *multi)
+enum mc_disposition do_multicall_call(struct multicall_entry *multi)
 {
     arm_hypercall_fn_t call = NULL;
 
     if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     call = arm_hypercall_table[multi->op].fn;
     if ( call == NULL )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     if ( is_32bit_domain(current->domain) &&
          !check_multicall_32bit_clean(multi) )
-        return;
+        return mc_continue;
 
     multi->result = call(multi->args[0], multi->args[1],
                          multi->args[2], multi->args[3],
                          multi->args[4]);
+
+    return likely(!psr_mode_is_user(guest_cpu_user_regs()))
+           ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -40,6 +40,7 @@ do_multicall(
     struct mc_state *mcs = &current->mc_state;
     uint32_t         i;
     int              rc = 0;
+    enum mc_disposition disp = mc_continue;
 
     if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
     {
@@ -50,7 +51,7 @@ do_multicall(
     if ( unlikely(!guest_handle_okay(call_list, nr_calls)) )
         rc = -EFAULT;
 
-    for ( i = 0; !rc && i < nr_calls; i++ )
+    for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ )
     {
         if ( i && hypercall_preempt_check() )
             goto preempted;
@@ -63,7 +64,7 @@ do_multicall(
 
         trace_multicall_call(&mcs->call);
 
-        do_multicall_call(&mcs->call);
+        disp = do_multicall_call(&mcs->call);
 
 #ifndef NDEBUG
         {
@@ -77,7 +78,14 @@ do_multicall(
         }
 #endif
 
-        if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
+        if ( unlikely(disp == mc_exit) )
+        {
+            if ( __copy_field_to_guest(call_list, &mcs->call, result) )
+                /* nothing, best effort only */;
+            rc = mcs->call.result;
+        }
+        else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call,
+                                                 result)) )
             rc = -EFAULT;
         else if ( mcs->flags & MCSF_call_preempted )
         {
@@ -93,6 +101,9 @@ do_multicall(
             guest_handle_add_offset(call_list, 1);
     }
 
+    if ( unlikely(disp == mc_preempt) && i < nr_calls )
+        goto preempted;
+
     perfc_incr(calls_to_multicall);
     perfc_add(calls_from_multicall, i);
     mcs->flags = 0;
--- a/xen/include/asm-arm/multicall.h
+++ b/xen/include/asm-arm/multicall.h
@@ -1,7 +1,11 @@
 #ifndef __ASM_ARM_MULTICALL_H__
 #define __ASM_ARM_MULTICALL_H__
 
-extern void do_multicall_call(struct multicall_entry *call);
+extern enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+} do_multicall_call(struct multicall_entry *call);
 
 #endif /* __ASM_ARM_MULTICALL_H__ */
 /*
--- a/xen/include/asm-x86/multicall.h
+++ b/xen/include/asm-x86/multicall.h
@@ -7,8 +7,21 @@
 
 #include <xen/errno.h>
 
+enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+};
+
+#define multicall_ret(call)                                  \
+    (unlikely((call)->op == __HYPERVISOR_iret)               \
+     ? mc_exit                                               \
+       : likely(guest_kernel_mode(current,                   \
+                                  guest_cpu_user_regs()))    \
+         ? mc_continue : mc_preempt)
+
 #define do_multicall_call(_call)                             \
-    do {                                                     \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movq  %c1(%0),%%rax; "                      \
             "    leaq  hypercall_table(%%rip),%%rdi; "       \
@@ -37,9 +50,11 @@
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
               "r8",  "r9",  "r10", "r11" );                  \
-    } while ( 0 )
+        multicall_ret(_call);                                \
+    })
 
 #define compat_multicall_call(_call)                         \
+    ({                                                       \
         __asm__ __volatile__ (                               \
             "    movl  %c1(%0),%%eax; "                      \
             "    leaq  compat_hypercall_table(%%rip),%%rdi; "\
@@ -67,6 +82,8 @@
               "i" (-ENOSYS)                                  \
               /* all the caller-saves registers */           \
             : "rax", "rcx", "rdx", "rsi", "rdi",             \
-              "r8",  "r9",  "r10", "r11" )                   \
+              "r8",  "r9",  "r10", "r11" );                  \
+        multicall_ret(_call);                                \
+    })
 
 #endif /* __ASM_X86_MULTICALL_H__ */

[-- Attachment #6: xsa213-4.8.patch --]
[-- Type: application/octet-stream, Size: 5628 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: multicall: deal with early exit conditions

In particular changes to guest privilege level require the multicall
sequence to be aborted, as hypercalls are permitted from kernel mode
only. While likely not very useful in a multicall, also properly handle
the return value in the HYPERVISOR_iret case (which should be the guest
specified value).

This is XSA-213.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1550,7 +1550,7 @@ static bool_t check_multicall_32bit_clea
     return true;
 }
 
-void arch_do_multicall_call(struct mc_state *state)
+enum mc_disposition arch_do_multicall_call(struct mc_state *state)
 {
     struct multicall_entry *multi = &state->call;
     arm_hypercall_fn_t call = NULL;
@@ -1558,23 +1558,26 @@ void arch_do_multicall_call(struct mc_st
     if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     call = arm_hypercall_table[multi->op].fn;
     if ( call == NULL )
     {
         multi->result = -ENOSYS;
-        return;
+        return mc_continue;
     }
 
     if ( is_32bit_domain(current->domain) &&
          !check_multicall_32bit_clean(multi) )
-        return;
+        return mc_continue;
 
     multi->result = call(multi->args[0], multi->args[1],
                          multi->args[2], multi->args[3],
                          multi->args[4]);
+
+    return likely(!psr_mode_is_user(guest_cpu_user_regs()))
+           ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -255,15 +255,19 @@ void pv_hypercall(struct cpu_user_regs *
     perfc_incr(hypercalls);
 }
 
-void arch_do_multicall_call(struct mc_state *state)
+enum mc_disposition arch_do_multicall_call(struct mc_state *state)
 {
-    if ( !is_pv_32bit_vcpu(current) )
+    struct vcpu *curr = current;
+    unsigned long op;
+
+    if ( !is_pv_32bit_vcpu(curr) )
     {
         struct multicall_entry *call = &state->call;
 
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].native )
-            call->result = pv_hypercall_table[call->op].native(
+        op = call->op;
+        if ( (op < ARRAY_SIZE(pv_hypercall_table)) &&
+             pv_hypercall_table[op].native )
+            call->result = pv_hypercall_table[op].native(
                 call->args[0], call->args[1], call->args[2],
                 call->args[3], call->args[4], call->args[5]);
         else
@@ -274,15 +278,21 @@ void arch_do_multicall_call(struct mc_st
     {
         struct compat_multicall_entry *call = &state->compat_call;
 
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].compat )
-            call->result = pv_hypercall_table[call->op].compat(
+        op = call->op;
+        if ( (op < ARRAY_SIZE(pv_hypercall_table)) &&
+             pv_hypercall_table[op].compat )
+            call->result = pv_hypercall_table[op].compat(
                 call->args[0], call->args[1], call->args[2],
                 call->args[3], call->args[4], call->args[5]);
         else
             call->result = -ENOSYS;
     }
 #endif
+
+    return unlikely(op == __HYPERVISOR_iret)
+           ? mc_exit
+           : likely(guest_kernel_mode(curr, guest_cpu_user_regs()))
+             ? mc_continue : mc_preempt;
 }
 
 /*
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -40,6 +40,7 @@ do_multicall(
     struct mc_state *mcs = &current->mc_state;
     uint32_t         i;
     int              rc = 0;
+    enum mc_disposition disp = mc_continue;
 
     if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
     {
@@ -50,7 +51,7 @@ do_multicall(
     if ( unlikely(!guest_handle_okay(call_list, nr_calls)) )
         rc = -EFAULT;
 
-    for ( i = 0; !rc && i < nr_calls; i++ )
+    for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ )
     {
         if ( i && hypercall_preempt_check() )
             goto preempted;
@@ -63,7 +64,7 @@ do_multicall(
 
         trace_multicall_call(&mcs->call);
 
-        arch_do_multicall_call(mcs);
+        disp = arch_do_multicall_call(mcs);
 
 #ifndef NDEBUG
         {
@@ -77,7 +78,14 @@ do_multicall(
         }
 #endif
 
-        if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
+        if ( unlikely(disp == mc_exit) )
+        {
+            if ( __copy_field_to_guest(call_list, &mcs->call, result) )
+                /* nothing, best effort only */;
+            rc = mcs->call.result;
+        }
+        else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call,
+                                                 result)) )
             rc = -EFAULT;
         else if ( mcs->flags & MCSF_call_preempted )
         {
@@ -93,6 +101,9 @@ do_multicall(
             guest_handle_add_offset(call_list, 1);
     }
 
+    if ( unlikely(disp == mc_preempt) && i < nr_calls )
+        goto preempted;
+
     perfc_incr(calls_to_multicall);
     perfc_add(calls_from_multicall, i);
     mcs->flags = 0;
--- a/xen/include/xen/multicall.h
+++ b/xen/include/xen/multicall.h
@@ -24,6 +24,10 @@ struct mc_state {
     };
 };
 
-void arch_do_multicall_call(struct mc_state *mc);
+enum mc_disposition {
+    mc_continue,
+    mc_exit,
+    mc_preempt,
+} arch_do_multicall_call(struct mc_state *mc);
 
 #endif /* __XEN_MULTICALL_H__ */

[-- Attachment #7: 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] only message in thread

only message in thread, other threads:[~2017-05-02 12:00 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-02 12:00 Xen Security Advisory 213 - x86: 64bit PV guest breakout via pagetable use-after-mode-change Xen.org security team

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.