From 4bf9b19260aefbb6a72f8f40121eb1c64595066a Mon Sep 17 00:00:00 2001 From: Keir Fraser Date: Thu, 12 May 2011 18:03:47 +0100 Subject: [PATCH] x86, vtd: [CVE-2011-1898] Protect against malicious MSIs from untrusted devices. In the absence of VT-d interrupt remapping support, a device can send arbitrary APIC messages to host CPUs. One class of attack that results is to confuse the hypervisor by delivering asynchronous interrupts to vectors that are expected to handle only synchronous traps/exceptions. We block this class of attack by: (1) setting APIC.TPR=0x10, to block all interrupts below vector 0x20. This blocks delivery to all architectural exception vectors. (2) checking APIC.ISR[vec] for vectors 0x80 (fast syscall) and 0x82 (hypercall). In these cases we BUG if we detect we are handling a hardware interrupt -- turning a potentially more severe infiltration into a straightforward system crash (i.e, DoS). Thanks to Invisible Things Lab for discovery and detailed investigation of this attack. Signed-off-by: Keir Fraser xen-unstable changeset: 23337:cc91832a02c7 xen-unstable date: Thu May 12 16:39:31 2011 +0100 --- xen/arch/x86/apic.c | 13 ++++++++----- xen/arch/x86/x86_64/compat/entry.S | 10 ++++++++++ xen/arch/x86/x86_64/entry.S | 8 ++++++++ xen/drivers/passthrough/vtd/iommu.c | 11 +++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c index 1590c3a17..3a0f6fb72 100644 --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -575,12 +575,9 @@ void __devinit setup_local_APIC(void) init_apic_ldr(); /* - * Set Task Priority to 'accept all'. We never change this - * later on. + * Set Task Priority to reject any interrupts below FIRST_DYNAMIC_VECTOR. */ - value = apic_read(APIC_TASKPRI); - value &= ~APIC_TPRI_MASK; - apic_write_around(APIC_TASKPRI, value); + apic_write_around(APIC_TASKPRI, (FIRST_DYNAMIC_VECTOR & 0xF0) - 0x10); /* * After a crash, we no longer service the interrupts and a pending @@ -1483,3 +1480,9 @@ int __init APIC_init_uniprocessor (void) return 0; } + +void check_for_unexpected_msi(unsigned int vector) +{ + unsigned long v = apic_read(APIC_ISR + ((vector & ~0x1f) >> 1)); + BUG_ON(v & (1 << (vector & 0x1f))); +} diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S index d87ef0cfd..8208b6b54 100644 --- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -10,12 +10,22 @@ #include #include #include +#include ALIGN ENTRY(compat_hypercall) pushq $0 movl $TRAP_syscall,4(%rsp) SAVE_ALL + + cmpb $0,untrusted_msi(%rip) +UNLIKELY_START(ne, msi_check) + movl $HYPERCALL_VECTOR,%edi + call check_for_unexpected_msi + RESTORE_ALL + SAVE_ALL +UNLIKELY_END(msi_check) + GET_CURRENT(%rbx) cmpl $NR_hypercalls,%eax diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index a6c68220c..61938f8f4 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -296,6 +296,14 @@ ENTRY(int80_direct_trap) pushq $0 SAVE_ALL + cmpb $0,untrusted_msi(%rip) +UNLIKELY_START(ne, msi_check) + movl $0x80,%edi + call check_for_unexpected_msi + RESTORE_ALL + SAVE_ALL +UNLIKELY_END(msi_check) + GET_CURRENT(%rbx) /* Check that the callback is non-null. */ diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index abcf02245..b33cff22e 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -48,6 +48,9 @@ static int sharept = 0; boolean_param("sharept", sharept); +/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */ +bool_t __read_mostly untrusted_msi; + int nr_iommus; static void setup_dom0_devices(struct domain *d); @@ -1582,6 +1585,14 @@ static int reassign_device_ownership( if (!pdev) return -ENODEV; + /* + * Devices assigned to untrusted domains (here assumed to be any domU) + * can attempt to send arbitrary LAPIC/MSI messages. We are unprotected + * by the root complex unless interrupt remapping is enabled. + */ + if ( (target != dom0) && !iommu_intremap ) + untrusted_msi = 1; + ret = domain_context_unmap(source, bus, devfn); if ( ret ) return ret; -- 2.39.5