[Pkg-xen-devel] Xen package security updates for jessie 4.4, XSA-213, XSA-214

Ian Jackson ian.jackson at eu.citrix.com
Thu May 4 19:18:07 UTC 2017


Moritz Muehlenhoff writes ("Re: Xen package security updates for jessie 4.4, XSA-213, XSA-214"):
> Yes, the distribution line should be jessie-security, but please send
> a debdiff to team at security.debian.org for a quick review before
> uploading (I have no idea whether dgit supports security-master).

Here is the proposed debdiff (actually, a git diff) for xen in jessie.

My ARM test build is still running but I think it's going to work.  I
have actually tested the i386 package.

Can I do a source-only upload ?

Ian.

diff --git a/debian/changelog b/debian/changelog
index 25361a61e4..a42f68d3a9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+xen (4.4.1-9+deb8u9) unstable; urgency=medium
+
+  Security updates:
+  * XSA-213: Closes:#861659: 64bit PV guest breakout
+  * XSA-214: Closes:#861660: grant transfer PV privilege escalation
+  * XSA-215: Closes:#861662: memory corruption via failsafe callback
+
+ -- Ian Jackson <ijackson at chiark.greenend.org.uk>  Thu, 04 May 2017 20:06:35 +0100
+
 xen (4.4.1-9+deb8u8) jessie-security; urgency=high
 
   * Non-maintainer upload by the Security Team.
diff --git a/debian/patches/multicall-deal-with-early-exit-condition b/debian/patches/multicall-deal-with-early-exit-condition
new file mode 100644
index 0000000000..853ab8a639
--- /dev/null
+++ b/debian/patches/multicall-deal-with-early-exit-condition
@@ -0,0 +1,182 @@
+From: Jan Beulich <jbeulich at suse.com>
+Date: Tue, 2 May 2017 11:32:26 -0500
+X-Dgit-Generated: 4.4.1-9+deb8u9 e54bb017f2f29a2130a9268b5eb0a6af8b56a567
+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 at google.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+Reviewed-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+Acked-by: Julien Grall <julien.grall at arm.com>
+
+Backported to Xen  4.4 for Centos
+From: Kevin Stange <kevin at steadfast.net>
+
+Dropped ARM psr_mode_is_user check.  Vulnerability is not present
+on ARM, anyway, according to upstream advisory.
+Signed-off-by: Ian Jackson <ian.jackson at eu.citrix.com>
+
+---
+
+--- xen-4.4.1.orig/xen/arch/arm/traps.c
++++ xen-4.4.1/xen/arch/arm/traps.c
+@@ -1243,30 +1243,31 @@ 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_pv32_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 mc_continue; /* XXX XSA-213 remains! */
+ }
+ 
+ /*
+--- xen-4.4.1.orig/xen/common/multicall.c
++++ xen-4.4.1/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;
+--- xen-4.4.1.orig/xen/include/asm-arm/multicall.h
++++ xen-4.4.1/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__ */
+ /*
+--- xen-4.4.1.orig/xen/include/asm-x86/multicall.h
++++ xen-4.4.1/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__ */
diff --git a/debian/patches/series b/debian/patches/series
index c2c8ee1e5f..59aa7d5938 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -110,3 +110,6 @@ CVE-2016-9382-xsa192-4.5.patch
 CVE-2016-9385-xsa193-4.5.patch
 CVE-2016-9383-xsa195.patch
 CVE-2016-9379_CVE-2016-9380-xsa198.patch
+multicall-deal-with-early-exit-condition
+x86-discard-type-information-when-steali
+x86-correct-create_bounce_frame
diff --git a/debian/patches/x86-correct-create_bounce_frame b/debian/patches/x86-correct-create_bounce_frame
new file mode 100644
index 0000000000..a3d6de2056
--- /dev/null
+++ b/debian/patches/x86-correct-create_bounce_frame
@@ -0,0 +1,41 @@
+From: Jan Beulich <jbeulich at suse.com>
+Date: Thu, 4 May 2017 18:28:33 +0100
+X-Dgit-Generated: 4.4.1-9+deb8u9 9bc3160aafebd907feaacfac1c8d228e7d7ba8a6
+Subject: x86: correct create_bounce_frame
+
+We may push up to 96 bytes on the guest (kernel) stack, so we should
+also cover as much in the early range check. Note that this is the
+simplest possible patch, which has the theoretical potential of
+breaking a guest: We only really push 96 bytes when invoking the
+failsafe callback, ordinary exceptions only have 56 or 64 bytes pushed
+(without / with error code respectively). There is, however, no PV OS
+known to place a kernel stack there.
+
+This is XSA-215.
+
+Reported-by: Jann Horn <jannh at google.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+Reviewed-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+
+---
+
+--- xen-4.4.1.orig/xen/arch/x86/x86_64/entry.S
++++ xen-4.4.1/xen/arch/x86/x86_64/entry.S
+@@ -346,7 +346,7 @@ int80_slow_path:
+         jmp   handle_exception_saved
+ 
+ /* CREATE A BASIC EXCEPTION FRAME ON GUEST OS STACK:                     */
+-/*   { RCX, R11, [DS-GS,] [CR2,] [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }   */
++/*   { RCX, R11, [DS-GS,] [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }          */
+ /* %rdx: trap_bounce, %rbx: struct vcpu                                  */
+ /* On return only %rbx and %rdx are guaranteed non-clobbered.            */
+ create_bounce_frame:
+@@ -366,7 +366,7 @@ create_bounce_frame:
+ 2:      andq  $~0xf,%rsi                # Stack frames are 16-byte aligned.
+         movq  $HYPERVISOR_VIRT_START,%rax
+         cmpq  %rax,%rsi
+-        movq  $HYPERVISOR_VIRT_END+60,%rax
++        movq  $HYPERVISOR_VIRT_END+12*8,%rax
+         sbb   %ecx,%ecx                 # In +ve address space? Then okay.
+         cmpq  %rax,%rsi
+         adc   %ecx,%ecx                 # Above Xen private area? Then okay.
diff --git a/debian/patches/x86-discard-type-information-when-steali b/debian/patches/x86-discard-type-information-when-steali
new file mode 100644
index 0000000000..d642370f22
--- /dev/null
+++ b/debian/patches/x86-discard-type-information-when-steali
@@ -0,0 +1,45 @@
+From: Jan Beulich <jbeulich at suse.com>
+Date: Thu, 4 May 2017 18:28:17 +0100
+X-Dgit-Generated: 4.4.1-9+deb8u9 15d93e7f2e3e5576049ccbf49e75c5c28dd02142
+Subject: x86: discard type information when stealing pages
+
+While a page having just a single general reference left necessarily
+has a zero type reference count too, its type may still be valid (and
+in validated state; at present this is only possible and relevant for
+PGT_seg_desc_page, as page tables have their type forcibly zapped when
+their type reference count drops to zero, and
+PGT_{writable,shared}_page pages don't require any validation). In
+such a case when the page is being re-used with the same type again,
+validation is being skipped. As validation criteria differ between
+32- and 64-bit guests, pages to be transferred between guests need to
+have their validation indicator zapped (and with it we zap all other
+type information at once).
+
+This is XSA-214.
+
+Reported-by: Jann Horn <jannh at google.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+Reviewed-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+
+---
+
+--- xen-4.4.1.orig/xen/arch/x86/mm.c
++++ xen-4.4.1/xen/arch/x86/mm.c
+@@ -4227,6 +4227,17 @@ int steal_page(
+         y = cmpxchg(&page->count_info, x, x & ~PGC_count_mask);
+     } while ( y != x );
+ 
++    /*
++     * With the sole reference dropped temporarily, no-one can update type
++     * information. Type count also needs to be zero in this case, but e.g.
++     * PGT_seg_desc_page may still have PGT_validated set, which we need to
++     * clear before transferring ownership (as validation criteria vary
++     * depending on domain type).
++     */
++    BUG_ON(page->u.inuse.type_info & (PGT_count_mask | PGT_locked |
++                                      PGT_pinned));
++    page->u.inuse.type_info = 0;
++
+     /* Swizzle the owner then reinstate the PGC_allocated reference. */
+     page_set_owner(page, NULL);
+     y = page->count_info;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4c910c810f..9491e662a1 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1243,30 +1243,31 @@ static bool_t check_multicall_32bit_clean(struct multicall_entry *multi)
     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_pv32_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 mc_continue; /* XXX XSA-213 remains! */
 }
 
 /*
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index ba13c4277e..209e9875db 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4227,6 +4227,17 @@ int steal_page(
         y = cmpxchg(&page->count_info, x, x & ~PGC_count_mask);
     } while ( y != x );
 
+    /*
+     * With the sole reference dropped temporarily, no-one can update type
+     * information. Type count also needs to be zero in this case, but e.g.
+     * PGT_seg_desc_page may still have PGT_validated set, which we need to
+     * clear before transferring ownership (as validation criteria vary
+     * depending on domain type).
+     */
+    BUG_ON(page->u.inuse.type_info & (PGT_count_mask | PGT_locked |
+                                      PGT_pinned));
+    page->u.inuse.type_info = 0;
+
     /* Swizzle the owner then reinstate the PGC_allocated reference. */
     page_set_owner(page, NULL);
     y = page->count_info;
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index c634217402..be973b3985 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -346,7 +346,7 @@ int80_slow_path:
         jmp   handle_exception_saved
 
 /* CREATE A BASIC EXCEPTION FRAME ON GUEST OS STACK:                     */
-/*   { RCX, R11, [DS-GS,] [CR2,] [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }   */
+/*   { RCX, R11, [DS-GS,] [ERRCODE,] RIP, CS, RFLAGS, RSP, SS }          */
 /* %rdx: trap_bounce, %rbx: struct vcpu                                  */
 /* On return only %rbx and %rdx are guaranteed non-clobbered.            */
 create_bounce_frame:
@@ -366,7 +366,7 @@ create_bounce_frame:
 2:      andq  $~0xf,%rsi                # Stack frames are 16-byte aligned.
         movq  $HYPERVISOR_VIRT_START,%rax
         cmpq  %rax,%rsi
-        movq  $HYPERVISOR_VIRT_END+60,%rax
+        movq  $HYPERVISOR_VIRT_END+12*8,%rax
         sbb   %ecx,%ecx                 # In +ve address space? Then okay.
         cmpq  %rax,%rsi
         adc   %ecx,%ecx                 # Above Xen private area? Then okay.
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index fa9d910594..da13573600 100644
--- 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;
diff --git a/xen/include/asm-arm/multicall.h b/xen/include/asm-arm/multicall.h
index b95926274f..ee3b345903 100644
--- 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__ */
 /*
diff --git a/xen/include/asm-x86/multicall.h b/xen/include/asm-x86/multicall.h
index a09ac5a1ae..32060aef38 100644
--- 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__ */



More information about the Pkg-xen-devel mailing list