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

[p***@xen.org]

commit f6b6ae78679b363ff670a9c125077c436dabd608
Author: Paul Durrant <***@citrix.com>
AuthorDate: Tue Nov 20 14:57:05 2018 +0100
Commit: Jan Beulich <***@suse.com>
CommitDate: Tue Nov 20 14:57:05 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 <***@arm.com>
Signed-off-by: Paul Durrant <***@citrix.com>
Signed-off-by: Jan Beulich <***@suse.com>
---
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 e2e755a8a1..da36ef727e 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -356,6 +356,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 )
{
@@ -378,27 +379,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;
}
@@ -406,15 +413,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