From: Paul Durrant <paul.durrant@citrix.com>
Date: Mon, 13 Jul 2015 09:56:36 +0000 (+0200)
Subject: x86/hvm: fix deadlock in emulation of rep mov to or from VRAM
X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~2832
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=046244423898d422c88c8961136fcd3eaf748d0f;p=xen.git

x86/hvm: fix deadlock in emulation of rep mov to or from VRAM

Razvan Cojocaru reported a hypervisor deadlock with the following stack:

(XEN)    [<ffff82d08012c3f1>] _spin_lock+0x31/0x54
(XEN)    [<ffff82d0801d09b6>] stdvga_mem_accept+0x3b/0x125
(XEN)    [<ffff82d0801cb23a>] hvm_find_io_handler+0x68/0x8a
(XEN)    [<ffff82d0801cb410>] hvm_mmio_internal+0x37/0x67
(XEN)    [<ffff82d0801c2403>] __hvm_copy+0xe9/0x37d
(XEN)    [<ffff82d0801c3e5d>] hvm_copy_from_guest_phys+0x14/0x16
(XEN)    [<ffff82d0801cb107>] hvm_process_io_intercept+0x10b/0x1d6
(XEN)    [<ffff82d0801cb291>] hvm_io_intercept+0x35/0x5b
(XEN)    [<ffff82d0801bb440>] hvmemul_do_io+0x1ff/0x2c1
(XEN)    [<ffff82d0801bc0b9>] hvmemul_do_io_addr+0x117/0x163
(XEN)    [<ffff82d0801bc129>] hvmemul_do_mmio_addr+0x24/0x26
(XEN)    [<ffff82d0801bcbb5>] hvmemul_rep_movs+0x1ef/0x335
(XEN)    [<ffff82d080198b49>] x86_emulate+0x56c9/0x13088
(XEN)    [<ffff82d0801bbd26>] _hvm_emulate_one+0x186/0x281
(XEN)    [<ffff82d0801bc1e8>] hvm_emulate_one+0x10/0x12
(XEN)    [<ffff82d0801cb63e>] handle_mmio+0x54/0xd2
(XEN)    [<ffff82d0801cb700>] handle_mmio_with_translation+0x44/0x46
(XEN)    [<ffff82d0801c27f6>] hvm_hap_nested_page_fault+0x15f/0x589
(XEN)    [<ffff82d0801e9741>] vmx_vmexit_handler+0x150e/0x188d
(XEN)    [<ffff82d0801ee7d1>] vmx_asm_vmexit_handler+0x41/0xc0

The problem here is the call to hvm_mmio_internal() being made by
__hvm_copy().

When the emulated VRAM access was originally started by
hvm_io_intercept() a few frames up the stack, it would have called
stdvga_mem_accept() which would then have acquired the per-domain
stdvga lock. Unfortunately the call to hvm_mmio_internal(), to avoid
a costly P2M walk, speculatively calls stdvga_mem_accept() again to
see if the page handed to __hvm_copy() is actually an internally
emulated page and hence the vcpu deadlocks.

The fix is to do the range-check in stdvga_mem_accept() without taking
the stdvga lock. This is safe because the range is constant and we know
the I/O will never actually be accepted by the stdvga device model
because hvmemul_do_io_addr() makes sure that the source of the I/O is
actually RAM.

Reported-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

diff --git a/xen/arch/x86/hvm/stdvga.c b/xen/arch/x86/hvm/stdvga.c
index 47b8432f73..6d22b224bb 100644
--- a/xen/arch/x86/hvm/stdvga.c
+++ b/xen/arch/x86/hvm/stdvga.c
@@ -492,11 +492,18 @@ static bool_t stdvga_mem_accept(const struct hvm_io_handler *handler,
 {
     struct hvm_hw_stdvga *s = &current->domain->arch.hvm_domain.stdvga;
 
+    /*
+     * The range check must be done without taking the lock, to avoid
+     * deadlock when hvm_mmio_internal() is called from
+     * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
+     */
+    if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
+         (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
+        return 0;
+
     spin_lock(&s->lock);
 
-    if ( !s->stdvga ||
-         (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
-         (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
+    if ( !s->stdvga )
         goto reject;
 
     if ( p->dir == IOREQ_WRITE && p->count > 1 )