[Xen-changelog] [xen staging-4.11] x86/hvm/ioreq: fix page referencing

[patchbot@xxxxxxx]

commit 3b2a779ccb9fd3c02ab2a68cb95a9628f0837029
Author:     Paul Durrant <paul.durrant@xxxxxxxxxx>
AuthorDate: Tue Nov 20 15:31:14 2018 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Nov 20 15:31:14 2018 +0100

    x86/hvm/ioreq: fix page referencing
    
    The code does not take a page reference in hvm_alloc_ioreq_mfn(), only a
    type reference. This can lead to a situation where a malicious domain with
    XSM_DM_PRIV can engineer a sequence as follows:
    
    - create IOREQ server: no pages as yet.
    - acquire resource: page allocated, total 0.
    - decrease reservation: -1 ref, total -1.
    
    This will cause Xen to hit a BUG_ON() in free_domheap_pages().
    
    This patch fixes the issue by changing the call to get_page_type() in
    hvm_alloc_ioreq_mfn() to a call to get_page_and_type(). This change
    in turn requires an extra put_page() in hvm_free_ioreq_mfn() in the case
    that _PGC_allocated is still set (i.e. a decrease reservation has not
    occurred) to avoid the page being leaked.
    
    This is part of XSA-276.
    
    Reported-by: Julien Grall <julien.grall@xxxxxxx>
    Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: f6b6ae78679b363ff670a9c125077c436dabd608
    master date: 2018-11-20 14:57:05 +0100
---
 xen/arch/x86/hvm/ioreq.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index f39f391929..bdc2687014 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -327,6 +327,7 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, 
bool buf)
 static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
 {
     struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+    struct page_info *page;
 
     if ( iorp->page )
     {
@@ -349,27 +350,33 @@ static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server 
*s, bool buf)
      * could fail if the emulating domain has already reached its
      * maximum allocation.
      */
-    iorp->page = alloc_domheap_page(s->emulator, MEMF_no_refcount);
+    page = alloc_domheap_page(s->emulator, MEMF_no_refcount);
 
-    if ( !iorp->page )
+    if ( !page )
         return -ENOMEM;
 
-    if ( !get_page_type(iorp->page, PGT_writable_page) )
-        goto fail1;
+    if ( !get_page_and_type(page, s->emulator, PGT_writable_page) )
+    {
+        /*
+         * The domain can't possibly know about this page yet, so failure
+         * here is a clear indication of something fishy going on.
+         */
+        domain_crash(s->emulator);
+        return -ENODATA;
+    }
 
-    iorp->va = __map_domain_page_global(iorp->page);
+    iorp->va = __map_domain_page_global(page);
     if ( !iorp->va )
-        goto fail2;
+        goto fail;
 
+    iorp->page = page;
     clear_page(iorp->va);
     return 0;
 
- fail2:
-    put_page_type(iorp->page);
-
- fail1:
-    put_page(iorp->page);
-    iorp->page = NULL;
+ fail:
+    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+        put_page(page);
+    put_page_and_type(page);
 
     return -ENOMEM;
 }
@@ -377,15 +384,24 @@ static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server 
*s, bool buf)
 static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
 {
     struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
+    struct page_info *page = iorp->page;
 
-    if ( !iorp->page )
+    if ( !page )
         return;
 
+    iorp->page = NULL;
+
     unmap_domain_page_global(iorp->va);
     iorp->va = NULL;
 
-    put_page_and_type(iorp->page);
-    iorp->page = NULL;
+    /*
+     * Check whether we need to clear the allocation reference before
+     * dropping the explicit references taken by get_page_and_type().
+     */
+    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+        put_page(page);
+
+    put_page_and_type(page);
 }
 
 bool is_ioreq_server_page(struct domain *d, const struct page_info *page)
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.11

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog