Ian Jackson
2017-May-04 16:59 UTC
[Pkg-xen-devel] Bug#861660: Xen package security updates for jessie 4.4, XSA-213, XSA-214
Moritz Muehlenhoff writes ("Re: Xen package security updates for jessie
4.4, XSA-213, XSA-214"):> On Thu, May 04, 2017 at 05:06:07PM +0100, Ian Jackson wrote:
> > I have fixed these in stretch but the jessie package remains unfixed.
> > I think I may be able to find some backports somewhere. Would that be
> > useful ? Is anyone else working on this ?
>
> Yes, please!
Working on it now. What shall I do with my resulting package ?
Should I put jessie-security in the debian/changelog and dgit push it
(ie, from many people's pov, dput it) ?
Ian.
Moritz Muehlenhoff
2017-May-04 17:06 UTC
[Pkg-xen-devel] Bug#861660: Xen package security updates for jessie 4.4, XSA-213, XSA-214
On Thu, May 04, 2017 at 05:59:18PM +0100, Ian Jackson wrote:> Moritz Muehlenhoff writes ("Re: Xen package security updates for jessie 4.4, XSA-213, XSA-214"): > > On Thu, May 04, 2017 at 05:06:07PM +0100, Ian Jackson wrote: > > > I have fixed these in stretch but the jessie package remains unfixed. > > > I think I may be able to find some backports somewhere. Would that be > > > useful ? Is anyone else working on this ? > > > > Yes, please! > > Working on it now. What shall I do with my resulting package ? > > Should I put jessie-security in the debian/changelog and dgit push it > (ie, from many people's pov, dput it) ?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). Cheers, Moritz
Ian Jackson
2017-May-04 17:19 UTC
[Pkg-xen-devel] Bug#861660: Xen package security updates for jessie 4.4, XSA-213, XSA-214
Moritz Muehlenhoff writes ("Re: Xen package security updates for jessie
4.4, XSA-213, XSA-214"):> On Thu, May 04, 2017 at 05:59:18PM +0100, Ian Jackson wrote:
> > Should I put jessie-security in the debian/changelog and dgit push it
> > (ie, from many people's pov, dput it) ?
>
> 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).
I'll send you a debdiff, thanks. I guess I'll find out whether dgit
does work or not.
I need to check the armhf build, since there are conflicts there. I
don't think I can conveniently test the armhf version.
Ian.
Ian Jackson
2017-May-04 19:18 UTC
[Pkg-xen-devel] Xen package security updates for jessie 4.4, XSA-213, XSA-214
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 = ¤t->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 = ¤t->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__ */