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

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 201 - ARM guests may induce host asynchronous abort
From:       Xen.org security team <security () xen ! org>
Date:       2016-11-29 14:48:20
Message-ID: E1cBjhk-0000Fg-Ct () xenbits ! xenproject ! org
[Download RAW message or body]

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

                    Xen Security Advisory XSA-201

             ARM guests may induce host asynchronous abort

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

Depending on how the hardware and firmware have been integrated,
guest-triggered asynchronous aborts (SError on ARMv8) may be received
by the hypervisor.  The current action is to crash the host.

A guest might trigger an asynchronous abort when accessing memory
mapped hardware in a non-conventional way.  Even if device
pass-through has not been configured, the hypervisor may give the
guest access to memory mapped hardware in order to take advantage of
hardware virtualization.

IMPACT
======

A malicious guest may be able to crash the host.

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

All Xen versions which support ARM are potentially affected.

Whether a particular ARM systems is affected depends on technical
details of the hardware and/or firmware.

x86 systems are not affected.

MITIGATION
==========

On systems where the guest kernel is controlled by the host rather than
guest administrator, running only kernels which do not expose MMIO to
userspace will prevent untrusted guest users from exploiting this issue.
However untrusted guest administrators can still trigger it unless
further steps are taken to prevent them 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.

NOTE REGARDING LACK OF EMBARGO
==============================

The issue was discussed publicly (and has been fixed already in KVM in
public trees).

CREDITS
=======

This issue was discovered by ARM engineering personnel.

RESOLUTION
==========

Applying the appropriate set of attached patched resolves this issue.

xsa201-[1234].patch       Xen-unstable

xsa201-[12].patch         }
xsa201-3-4.7.patch        } Xen 4.7.x, Xen 4.6.x
xsa201-4.patch            }

$ sha256sum xsa201*
ffdefdaa67748df7fccbc82011202724c622ca432cd121853ecab45ff4657406  xsa201-1.patch
0665eb575b056f98d5330ef23f497b2b3de1a15319e2012005890a17df32a7ed  xsa201-2.patch
4486d5efb59c1f1fff04a3cb697f948d5bf680e2a1c0d76cd44382ad8fa9095e  xsa201-3.patch
ca82c82acd51bf3cb8114d1843519c28e3df26243bd45eb712ff10ba11061b93  xsa201-3-4.7.patch
1de6ddb4b5b46ae390ec4587e588c00a706f4a68365d379db7ad54234f770d48  xsa201-4.patch
$
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJYPZSoAAoJEIP+FMlX6CvZ2zoH/ivzE70xsLHYJUxveoBiFuiU
KHFzF0X63G681FjLyU4SY2GkH5K9YutJ1uaakp+peD96fQqCXBHxWUMPAfblnd7t
YueMYuFqcz3mE2ypJjBh/fdI8a4UrKHHg3z6Hw6X91p+SRmPsnt9v7OzytoYOiE4
fDeaATwl1LxB+Z/yJETlo/JMgwrtuYZ9EZM9gIzxdOVw+QbQyEYHmuIyni8BNRvZ
+biRRQo37K5+jLY3f/RoXKcpqnHqjKOOmfjkxJJAsxqpdTSw5fRJqSZE4G5oUVs2
AAvSKhLObFahMlPqtoNXSC6lG5Gbd3e/h+6N2N/96TXs6Wr+d0VuC+lkYUjwcJk=
=KEYF
-----END PGP SIGNATURE-----

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

From: Wei Chen <Wei.Chen@arm.com>
Subject: arm64: handle guest-generated EL1 asynchronous abort

In current code, when the hypervisor receives an asynchronous abort
from a guest, the hypervisor will do panic, the host will be down.
We have to prevent such security issue, so, in this patch we crash
the guest, when the hypervisor receives an asynchronous abort from
the guest.

This is part of XSA-201.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Julien Grall <Julien.Grall@arm.com>

--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -204,9 +204,12 @@ guest_fiq_invalid:
         entry   hyp=0, compat=0
         invalid BAD_FIQ

-guest_error_invalid:
+guest_error:
         entry   hyp=0, compat=0
-        invalid BAD_ERROR
+        msr     daifclr, #2
+        mov     x0, sp
+        bl      do_trap_guest_error
+        exit    hyp=0, compat=0

 guest_sync_compat:
         entry   hyp=0, compat=1
@@ -225,9 +228,12 @@ guest_fiq_invalid_compat:
         entry   hyp=0, compat=1
         invalid BAD_FIQ

-guest_error_invalid_compat:
+guest_error_compat:
         entry   hyp=0, compat=1
-        invalid BAD_ERROR
+        msr     daifclr, #2
+        mov     x0, sp
+        bl      do_trap_guest_error
+        exit    hyp=0, compat=1

 ENTRY(return_to_new_vcpu32)
         exit    hyp=0, compat=1
@@ -286,12 +292,12 @@ ENTRY(hyp_traps_vector)
         ventry  guest_sync                      // Synchronous 64-bit EL0/EL1
         ventry  guest_irq                       // IRQ 64-bit EL0/EL1
         ventry  guest_fiq_invalid               // FIQ 64-bit EL0/EL1
-        ventry  guest_error_invalid             // Error 64-bit EL0/EL1
+        ventry  guest_error                     // Error 64-bit EL0/EL1

         ventry  guest_sync_compat               // Synchronous 32-bit EL0/EL1
         ventry  guest_irq_compat                // IRQ 32-bit EL0/EL1
         ventry  guest_fiq_invalid_compat        // FIQ 32-bit EL0/EL1
-        ventry  guest_error_invalid_compat      // Error 32-bit EL0/EL1
+        ventry  guest_error_compat              // Error 32-bit EL0/EL1

 /*
  * struct vcpu *__context_switch(struct vcpu *prev, struct vcpu *next)
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2723,6 +2723,21 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
     }
 }

+asmlinkage void do_trap_guest_error(struct cpu_user_regs *regs)
+{
+    enter_hypervisor_head(regs);
+
+    /*
+     * Currently, to ensure hypervisor safety, when we received a
+     * guest-generated vSerror/vAbort, we just crash the guest to protect
+     * the hypervisor. In future we can better handle this by injecting
+     * a vSerror/vAbort to the guest.
+     */
+    gdprintk(XENLOG_WARNING, "Guest(Dom-%u) will be crashed by vSError\n",
+             current->domain->domain_id);
+    domain_crash_synchronous();
+}
+
 asmlinkage void do_trap_irq(struct cpu_user_regs *regs)
 {
     enter_hypervisor_head(regs);

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

From: Wei Chen <Wei.Chen@arm.com>
Subject: arm64: handle async aborts delivered while at EL2

If EL1 generates an asynchronous abort and then traps into EL2
(by HVC or IRQ) before the abort has been delivered, the hypervisor
could not catch it, because the PSTATE.A bit is masked all the time
in hypervisor. So this asynchronous abort may be slipped to next
running guest with PSTATE.A bit unmasked.

In order to avoid this, it is necessary to take the abort at EL2, by
clearing the PSTATE.A bit. In this patch, we unmask the PSTATE.A bit
to open a window to catch guest-generated asynchronous abort in all
EL1 -> EL2 swich paths. If we catched such asynchronous abort in
checking window, the hyp_error exception will be triggered and the
abort source guest will be crashed.

This is part of XSA-201.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -173,6 +173,43 @@ hyp_error_invalid:
         entry   hyp=1
         invalid BAD_ERROR

+hyp_error:
+        /*
+         * Only two possibilities:
+         * 1) Either we come from the exit path, having just unmasked
+         *    PSTATE.A: change the return code to an EL2 fault, and
+         *    carry on, as we're already in a sane state to handle it.
+         * 2) Or we come from anywhere else, and that's a bug: we panic.
+         */
+        entry   hyp=1
+        msr     daifclr, #2
+
+        /*
+         * The ELR_EL2 may be modified by an interrupt, so we have to use the
+         * saved value in cpu_user_regs to check whether we come from 1) or
+         * not.
+         */
+        ldr     x0, [sp, #UREGS_PC]
+        adr     x1, abort_guest_exit_start
+        cmp     x0, x1
+        adr     x1, abort_guest_exit_end
+        ccmp    x0, x1, #4, ne
+        mov     x0, sp
+        mov     x1, #BAD_ERROR
+
+        /*
+         * Not equal, the exception come from 2). It's a bug, we have to
+         * panic the hypervisor.
+         */
+        b.ne    do_bad_mode
+
+        /*
+         * Otherwise, the exception come from 1). It happened because of
+         * the guest. Crash this guest.
+         */
+        bl      do_trap_guest_error
+        exit    hyp=1
+
 /* Traps taken in Current EL with SP_ELx */
 hyp_sync:
         entry   hyp=1
@@ -189,15 +226,29 @@ hyp_irq:

 guest_sync:
         entry   hyp=0, compat=0
+        bl      check_pending_vserror
+        /*
+         * If x0 is Non-zero, a vSError took place, the initial exception
+         * doesn't have any significance to be handled. Exit ASAP
+         */
+        cbnz    x0, 1f
         msr     daifclr, #2
         mov     x0, sp
         bl      do_trap_hypervisor
+1:
         exit    hyp=0, compat=0

 guest_irq:
         entry   hyp=0, compat=0
+        bl      check_pending_vserror
+        /*
+         * If x0 is Non-zero, a vSError took place, the initial exception
+         * doesn't have any significance to be handled. Exit ASAP
+         */
+        cbnz    x0, 1f
         mov     x0, sp
         bl      do_trap_irq
+1:
         exit    hyp=0, compat=0

 guest_fiq_invalid:
@@ -213,15 +264,29 @@ guest_error:

 guest_sync_compat:
         entry   hyp=0, compat=1
+        bl      check_pending_vserror
+        /*
+         * If x0 is Non-zero, a vSError took place, the initial exception
+         * doesn't have any significance to be handled. Exit ASAP
+         */
+        cbnz    x0, 1f
         msr     daifclr, #2
         mov     x0, sp
         bl      do_trap_hypervisor
+1:
         exit    hyp=0, compat=1

 guest_irq_compat:
         entry   hyp=0, compat=1
+        bl      check_pending_vserror
+        /*
+         * If x0 is Non-zero, a vSError took place, the initial exception
+         * doesn't have any significance to be handled. Exit ASAP
+         */
+        cbnz    x0, 1f
         mov     x0, sp
         bl      do_trap_irq
+1:
         exit    hyp=0, compat=1

 guest_fiq_invalid_compat:
@@ -270,6 +335,62 @@ return_from_trap:
         eret

 /*
+ * This function is used to check pending virtual SError in the gap of
+ * EL1 -> EL2 world switch.
+ * The x0 register will be used to indicate the results of detection.
+ * x0 -- Non-zero indicates a pending virtual SError took place.
+ * x0 -- Zero indicates no pending virtual SError took place.
+ */
+check_pending_vserror:
+        /*
+         * Save elr_el2 to check whether the pending SError exception takes
+         * place while we are doing this sync exception.
+         */
+        mrs     x0, elr_el2
+
+        /* Synchronize against in-flight ld/st */
+        dsb     sy
+
+        /*
+         * Unmask PSTATE asynchronous abort bit. If there is a pending
+         * SError, the EL2 error exception will happen after PSTATE.A
+         * is cleared.
+         */
+        msr     daifclr, #4
+
+        /*
+         * This is our single instruction exception window. A pending
+         * SError is guaranteed to occur at the earliest when we unmask
+         * it, and at the latest just after the ISB.
+         *
+         * If a pending SError occurs, the program will jump to EL2 error
+         * exception handler, and the elr_el2 will be set to
+         * abort_guest_exit_start or abort_guest_exit_end.
+         */
+abort_guest_exit_start:
+
+        isb
+
+abort_guest_exit_end:
+        /* Mask PSTATE asynchronous abort bit, close the checking window. */
+        msr     daifset, #4
+
+        /*
+         * Compare elr_el2 and the saved value to check whether we are
+         * returning from a valid exception caused by pending SError.
+         */
+        mrs     x1, elr_el2
+        cmp     x0, x1
+
+        /*
+         * Not equal, the pending SError exception took place, set
+         * x0 to non-zero.
+         */
+        cset    x0, ne
+
+        ret
+
+/*
  * Exception vectors.
  */
         .macro  ventry  label
@@ -287,7 +408,7 @@ ENTRY(hyp_traps_vector)
         ventry  hyp_sync                        // Synchronous EL2h
         ventry  hyp_irq                         // IRQ EL2h
         ventry  hyp_fiq_invalid                 // FIQ EL2h
-        ventry  hyp_error_invalid               // Error EL2h
+        ventry  hyp_error                       // Error EL2h

         ventry  guest_sync                      // Synchronous 64-bit EL0/EL1
         ventry  guest_irq                       // IRQ 64-bit EL0/EL1

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

From: Wei Chen <Wei.Chen@arm.com>
Subject: arm: crash the guest when it traps on external abort

If we spot a data or prefetch abort bearing the ESR_EL2.EA bit set, we
know that this is an external abort, and that should crash the guest.

This is part of XSA-201.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Julien Grall <Julien.Grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2409,6 +2409,15 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     paddr_t gpa;
     mfn_t mfn;

+    /*
+     * If this bit has been set, it means that this instruction abort is caused
+     * by a guest external abort. Currently we crash the guest to protect the
+     * hypervisor. In future one can better handle this by injecting a virtual
+     * abort to the guest.
+     */
+    if ( hsr.iabt.eat )
+        domain_crash_synchronous();
+
     if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
         gpa = get_faulting_ipa(gva);
     else
@@ -2503,6 +2512,15 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     uint8_t fsc = hsr.dabt.dfsc & ~FSC_LL_MASK;
     mfn_t mfn;

+    /*
+     * If this bit has been set, it means that this data abort is caused
+     * by a guest external abort. Currently we crash the guest to protect the
+     * hypervisor. In future one can better handle this by injecting a virtual
+     * abort to the guest.
+     */
+    if ( dabt.eat )
+        domain_crash_synchronous();
+
     info.dabt = dabt;
 #ifdef CONFIG_ARM_32
     info.gva = READ_CP32(HDFAR);

["xsa201-3-4.7.patch" (application/octet-stream)]

From: Wei Chen <Wei.Chen@arm.com>
Subject: arm: crash the guest when it traps on external abort

If we spot a data or prefetch abort bearing the ESR_EL2.EA bit set, we
know that this is an external abort, and that should crash the guest.

This is part of XSA-201.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Julien Grall <Julien.Grall@arm.com>

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2383,6 +2383,15 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     int rc;
     register_t gva = READ_SYSREG(FAR_EL2);
 
+    /*
+     * If this bit has been set, it means that this instruction abort is caused
+     * by a guest external abort. Currently we crash the guest to protect the
+     * hypervisor. In future one can better handle this by injecting a virtual
+     * abort to the guest.
+     */
+    if ( hsr.iabt.eat )
+        domain_crash_synchronous();
+
     switch ( hsr.iabt.ifsc & 0x3f )
     {
     case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
@@ -2437,6 +2446,15 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
         return;
     }
 
+    /*
+     * If this bit has been set, it means that this data abort is caused
+     * by a guest external abort. Currently we crash the guest to protect the
+     * hypervisor. In future one can better handle this by injecting a virtual
+     * abort to the guest.
+     */
+    if ( dabt.eat )
+        domain_crash_synchronous();
+
     info.dabt = dabt;
 #ifdef CONFIG_ARM_32
     info.gva = READ_CP32(HDFAR);

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

From: Wei Chen <Wei.Chen@arm.com>
Subject: arm32: handle async aborts delivered while at HYP

If guest generates an asynchronous abort and then traps into HYP
(by HVC or IRQ) before the abort has been delivered, the hypervisor
could not catch it, because the PSTATE.A bit is masked all the time
in hypervisor. So this asynchronous abort may be slipped to next
running guest with PSTATE.A bit unmasked.

In order to avoid this, it is necessary to take the abort at HYP, by
clearing the PSTATE.A bit. In this patch, we unmask the PSTATE.A bit
to open a window to catch guest-generated asynchronous abort in all
Guest -> HYP switch paths. If we caught such asynchronous abort in
checking window, the HYP data abort exception will be triggered and
the abort source guest will be crashed.

This is part of XSA-201.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>

--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -42,6 +42,61 @@ save_guest_regs:
         SAVE_BANKED(fiq)
         SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq); SAVE_ONE_BANKED(R10_fiq)
         SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
+        /*
+         * Start to check pending virtual abort in the gap of Guest -> HYP
+         * world switch.
+         *
+         * Save ELR_hyp to check whether the pending virtual abort exception
+         * takes place while we are doing this trap exception.
+         */
+        mrs r1, ELR_hyp
+
+        /*
+         * Force loads and stores to complete before unmasking asynchronous
+         * aborts and forcing the delivery of the exception.
+         */
+        dsb sy
+
+        /*
+         * Unmask asynchronous abort bit. If there is a pending asynchronous
+         * abort, the data_abort exception will happen after A bit is cleared.
+         */
+        cpsie a
+
+        /*
+         * This is our single instruction exception window. A pending
+         * asynchronous abort is guaranteed to occur at the earliest when we
+         * unmask it, and at the latest just after the ISB.
+         *
+         * If a pending abort occurs, the program will jump to data_abort
+         * exception handler, and the ELR_hyp will be set to
+         * abort_guest_exit_start or abort_guest_exit_end.
+         */
+        .global abort_guest_exit_start
+abort_guest_exit_start:
+
+        isb
+
+        .global abort_guest_exit_end
+abort_guest_exit_end:
+        /* Mask CPSR asynchronous abort bit, close the checking window. */
+        cpsid a
+
+        /*
+         * Compare ELR_hyp and the saved value to check whether we are
+         * returning from a valid exception caused by pending virtual
+         * abort.
+         */
+        mrs r2, ELR_hyp
+        cmp r1, r2
+
+        /*
+         * Not equal, the pending virtual abort exception took place, the
+         * initial exception does not have any significance to be handled.
+         * Exit ASAP.
+         */
+        bne return_from_trap
+
         mov pc, lr

 #define DEFINE_TRAP_ENTRY(trap)                                         \
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -63,7 +63,10 @@ asmlinkage void do_trap_prefetch_abort(struct cpu_user_regs *regs)

 asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
 {
-    do_unexpected_trap("Data Abort", regs);
+    if ( VABORT_GEN_BY_GUEST(regs) )
+        do_trap_guest_error(regs);
+    else
+        do_unexpected_trap("Data Abort", regs);
 }

 /*
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -55,6 +55,17 @@ struct cpu_user_regs

     uint32_t pad1; /* Doubleword-align the user half of the frame */
 };
+
+/* Functions for pending virtual abort checking window. */
+void abort_guest_exit_start(void);
+void abort_guest_exit_end(void);
+
+#define VABORT_GEN_BY_GUEST(r)  \
+( \
+    ( (unsigned long)abort_guest_exit_start == (r)->pc ) || \
+    ( (unsigned long)abort_guest_exit_end == (r)->pc ) \
+)
+
 #endif

 /* Layout as used in assembly, with src/dest registers mixed in */
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -690,6 +690,8 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
 int call_smc(register_t function_id, register_t arg0, register_t arg1,
              register_t arg2);

+void do_trap_guest_error(struct cpu_user_regs *regs);
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_ARM_PROCESSOR_H */
 /*


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

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