[PATCH] x86/hvm: Fix double free from vlapic_init() early error path

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

vlapic_init()'s caller calls vlapic_destroy() on error.  Therefore, the error
path from __map_domain_page_global() failing would doubly free
vlapic->regs_page.

Rework vlapic_destroy() to be properly idempotent, introducing the necessary
UNMAP_DOMAIN_PAGE_GLOBAL() and FREE_DOMHEAP_PAGE() wrappers.

Rearrange vlapic_init() to group all trivial initialisation, and leave all
cleanup to the caller, in line with our longer term plans.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
---
 xen/arch/x86/hvm/vlapic.c     | 11 ++++-------
 xen/include/xen/domain_page.h |  5 +++++
 xen/include/xen/mm.h          |  6 ++++++
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 5e21fb4937..da030f41b5 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1608,7 +1608,9 @@ int vlapic_init(struct vcpu *v)
         return 0;
     }
 
+    /* Trivial init. */
     vlapic->pt.source = PTSRC_lapic;
+    spin_lock_init(&vlapic->esr_lock);
 
     vlapic->regs_page = alloc_domheap_page(v->domain, MEMF_no_owner);
     if ( !vlapic->regs_page )
@@ -1616,17 +1618,12 @@ int vlapic_init(struct vcpu *v)
 
     vlapic->regs = __map_domain_page_global(vlapic->regs_page);
     if ( vlapic->regs == NULL )
-    {
-        free_domheap_page(vlapic->regs_page);
         return -ENOMEM;
-    }
 
     clear_page(vlapic->regs);
 
     vlapic_reset(vlapic);
 
-    spin_lock_init(&vlapic->esr_lock);
-
     tasklet_init(&vlapic->init_sipi.tasklet, vlapic_init_sipi_action, v);
 
     if ( v->vcpu_id == 0 )
@@ -1645,8 +1642,8 @@ void vlapic_destroy(struct vcpu *v)
     tasklet_kill(&vlapic->init_sipi.tasklet);
     TRACE_0D(TRC_HVM_EMUL_LAPIC_STOP_TIMER);
     destroy_periodic_time(&vlapic->pt);
-    unmap_domain_page_global(vlapic->regs);
-    free_domheap_page(vlapic->regs_page);
+    UNMAP_DOMAIN_PAGE_GLOBAL(vlapic->regs);
+    FREE_DOMHEAP_PAGE(vlapic->regs_page);
 }
 
 /*
diff --git a/xen/include/xen/domain_page.h b/xen/include/xen/domain_page.h
index a182d33b67..0cb7f2aad3 100644
--- a/xen/include/xen/domain_page.h
+++ b/xen/include/xen/domain_page.h
@@ -77,4 +77,9 @@ static inline void unmap_domain_page_global(const void *va) 
{};
     (p) = NULL;                     \
 } while ( false )
 
+#define UNMAP_DOMAIN_PAGE_GLOBAL(p) do {   \
+    unmap_domain_page_global(p);           \
+    (p) = NULL;                            \
+} while ( false )
+
 #endif /* __XEN_DOMAIN_PAGE_H__ */
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..c274e2eac4 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,12 @@ bool scrub_free_pages(void);
 } while ( false )
 #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
 
+#define FREE_DOMHEAP_PAGES(p, o) do { \
+    free_domheap_pages(p, o);         \
+    (p) = NULL;                       \
+} while ( false )
+#define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0)
+
 /* Map machine page range in Xen virtual address space. */
 int map_pages_to_xen(
     unsigned long virt,
-- 
2.11.0