[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen staging] x86/hvm/ioreq: fix page referencing
commit f6b6ae78679b363ff670a9c125077c436dabd608 Author: Paul Durrant <paul.durrant@xxxxxxxxxx> AuthorDate: Tue Nov 20 14:57:05 2018 +0100 Commit: Jan Beulich <jbeulich@xxxxxxxx> 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 <julien.grall@xxxxxxx> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- 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 _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |