Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:Update
xen.7317
594917ec-guest_physmap_remove_page-needs-its-re...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 594917ec-guest_physmap_remove_page-needs-its-return-value-checked.patch of Package xen.7317
# Commit a0cce6048d010a30ac82f8db7787bbf9aada64f4 # Date 2017-06-20 14:41:16 +0200 # Author Jan Beulich <jbeulich@suse.com> # Committer Jan Beulich <jbeulich@suse.com> guest_physmap_remove_page() needs its return value checked Callers, namely such subsequently freeing the page, must not blindly assume success - the function may namely fail when needing to shatter a super page, but there not being memory available for the then needed intermediate page table. As it happens, guest_remove_page() callers now also all check the return value. Furthermore a missed put_gfn() on an error path in gnttab_transfer() is also being taken care of. This is part of XSA-222. Reported-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1220,13 +1220,14 @@ int replace_grant_host_mapping(unsigned { unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT); struct domain *d = current->domain; + int rc; if ( new_addr != 0 || (flags & GNTMAP_contains_pte) ) return GNTST_general_error; - guest_physmap_remove_page(d, gfn, mfn, 0); + rc = guest_physmap_remove_page(d, gfn, mfn, 0); - return GNTST_okay; + return rc ? GNTST_general_error : GNTST_okay; } int is_iomem_page(unsigned long mfn) --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -501,14 +501,13 @@ int guest_physmap_add_entry(struct domai pfn_to_paddr(mfn), MATTR_MEM, t); } -void guest_physmap_remove_page(struct domain *d, - unsigned long gpfn, - unsigned long mfn, unsigned int page_order) +int guest_physmap_remove_page(struct domain *d, unsigned long gfn, + unsigned long mfn, unsigned int page_order) { - apply_p2m_changes(d, REMOVE, - pfn_to_paddr(gpfn), - pfn_to_paddr(gpfn + (1<<page_order)), - pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid); + return apply_p2m_changes(d, REMOVE, + pfn_to_paddr(gfn), + pfn_to_paddr(gfn + (1 << page_order)), + pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid); } int p2m_alloc_table(struct domain *d) --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4094,7 +4094,11 @@ static int replace_grant_p2m_mapping( type, mfn_x(old_mfn), frame); return GNTST_general_error; } - guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K); + if ( guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K) ) + { + put_gfn(d, gfn); + return GNTST_general_error; + } put_gfn(d, gfn); return GNTST_okay; @@ -4617,7 +4621,7 @@ int xenmem_add_to_physmap_one( struct page_info *page = NULL; unsigned long gfn = 0; /* gcc ... */ unsigned long prev_mfn, mfn = 0, old_gpfn; - int rc; + int rc = 0; p2m_type_t p2mt; switch ( space ) @@ -4689,25 +4693,30 @@ int xenmem_add_to_physmap_one( { if ( is_xen_heap_mfn(prev_mfn) ) /* Xen heap frames are simply unhooked from this phys slot. */ - guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K); + rc = guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K); else /* Normal domain memory is freed, to avoid leaking memory. */ - guest_remove_page(d, gpfn); + rc = guest_remove_page(d, gpfn); } /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */ put_gfn(d, gpfn); + if ( rc ) + goto put_both; + /* Unmap from old location, if any. */ old_gpfn = get_gpfn_from_mfn(mfn); ASSERT( old_gpfn != SHARED_M2P_ENTRY ); if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range ) ASSERT( old_gpfn == gfn ); if ( old_gpfn != INVALID_M2P_ENTRY ) - guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K); + rc = guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K); /* Map at new location. */ - rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K); + if ( !rc ) + rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K); + put_both: /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */ if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range ) put_gfn(d, gfn); --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -504,7 +504,7 @@ void p2m_final_teardown(struct domain *d } -static void +static int p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn, unsigned int page_order) { @@ -518,7 +518,7 @@ p2m_remove_page(struct p2m_domain *p2m, if ( need_iommu(p2m->domain) ) for ( i = 0; i < (1 << page_order); i++ ) iommu_unmap_page(p2m->domain, mfn + i); - return; + return 0; } ASSERT(gfn_locked_by_me(p2m, gfn)); @@ -534,17 +534,22 @@ p2m_remove_page(struct p2m_domain *p2m, ASSERT( !p2m_is_valid(t) || mfn + i == mfn_x(mfn_return) ); } } - set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), page_order, p2m_invalid, p2m->default_access); + return set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), page_order, + p2m_invalid, p2m->default_access) ? 0 : -ENOMEM; } -void +int guest_physmap_remove_page(struct domain *d, unsigned long gfn, unsigned long mfn, unsigned int page_order) { + int rc; + struct p2m_domain *p2m = p2m_get_hostp2m(d); gfn_lock(p2m, gfn, page_order); - p2m_remove_page(p2m, gfn, mfn, page_order); + rc = p2m_remove_page(p2m, gfn, mfn, page_order); gfn_unlock(p2m, gfn, page_order); + + return rc; } int --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -1529,6 +1529,7 @@ gnttab_transfer( for ( i = 0; i < count; i++ ) { bool_t okay; + int rc; if (i && hypercall_preempt_check()) return i; @@ -1579,27 +1580,33 @@ gnttab_transfer( goto copyback; } - guest_physmap_remove_page(d, gop.mfn, mfn, 0); + rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0); flush_tlb_mask(d->domain_dirty_cpumask); + if ( rc ) + { + gdprintk(XENLOG_INFO, + "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n", + gop.mfn, mfn); + gop.status = GNTST_general_error; + goto put_gfn_and_copyback; + } /* Find the target domain. */ if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) ) { - put_gfn(d, gop.mfn); gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n", gop.domid); - page->count_info &= ~(PGC_count_mask|PGC_allocated); - free_domheap_page(page); gop.status = GNTST_bad_domain; - goto copyback; + goto put_gfn_and_copyback; } if ( xsm_grant_transfer(XSM_HOOK, d, e) ) { - put_gfn(d, gop.mfn); gop.status = GNTST_permission_denied; unlock_and_copyback: rcu_unlock_domain(e); + put_gfn_and_copyback: + put_gfn(d, gop.mfn); page->count_info &= ~(PGC_count_mask|PGC_allocated); free_domheap_page(page); goto copyback; @@ -1650,12 +1657,8 @@ gnttab_transfer( "Transferee (d%d) has no headroom (tot %u, max %u)\n", e->domain_id, e->tot_pages, e->max_pages); - rcu_unlock_domain(e); - put_gfn(d, gop.mfn); - page->count_info &= ~(PGC_count_mask|PGC_allocated); - free_domheap_page(page); gop.status = GNTST_general_error; - goto copyback; + goto unlock_and_copyback; } /* Okay, add the page to 'e'. */ @@ -1684,13 +1687,8 @@ gnttab_transfer( if ( drop_dom_ref ) put_domain(e); - rcu_unlock_domain(e); - - put_gfn(d, gop.mfn); - page->count_info &= ~(PGC_count_mask|PGC_allocated); - free_domheap_page(page); gop.status = GNTST_general_error; - goto copyback; + goto unlock_and_copyback; } page_list_add_tail(page, &e->page_list); --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -248,8 +248,12 @@ int guest_remove_page(struct domain *d, mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); if ( unlikely(p2m_is_paging(p2mt)) ) { - guest_physmap_remove_page(d, gmfn, mfn, 0); + rc = guest_physmap_remove_page(d, gmfn, mfn, 0); put_gfn(d, gmfn); + + if ( rc ) + return rc; + /* If the page hasn't yet been paged out, there is an * actual page that needs to be released. */ if ( p2mt == p2m_ram_paging_out ) @@ -306,18 +310,18 @@ int guest_remove_page(struct domain *d, return -ENXIO; } - if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) ) + rc = guest_physmap_remove_page(d, gmfn, mfn, 0); + + if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) ) put_page_and_type(page); - if ( test_and_clear_bit(_PGC_allocated, &page->count_info) ) + if ( !rc && test_and_clear_bit(_PGC_allocated, &page->count_info) ) put_page(page); - guest_physmap_remove_page(d, gmfn, mfn, 0); - put_page(page); put_gfn(d, gmfn); - return 0; + return rc; } static void decrease_reservation(struct memop_args *a) @@ -558,7 +562,8 @@ static long memory_exchange(XEN_GUEST_HA gfn = mfn_to_gmfn(d, mfn); /* Pages were unshared above */ BUG_ON(SHARED_M2P(gfn)); - guest_physmap_remove_page(d, gfn, mfn, 0); + if ( guest_physmap_remove_page(d, gfn, mfn, 0) ) + domain_crash(d); put_page(page); } @@ -1003,7 +1008,7 @@ long do_memory_op(unsigned long cmd, XEN page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC); if ( page ) { - guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0); + rc = guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0); put_page(page); } else --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -108,10 +108,6 @@ static inline int guest_physmap_add_page return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw); } -void guest_physmap_remove_page(struct domain *d, - unsigned long gpfn, - unsigned long mfn, unsigned int page_order); - unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn); /* --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -501,11 +501,6 @@ static inline int guest_physmap_add_page return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw); } -/* Remove a page from a domain's p2m table */ -void guest_physmap_remove_page(struct domain *d, - unsigned long gfn, - unsigned long mfn, unsigned int page_order); - /* Set a p2m range as populate-on-demand */ int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn, unsigned int order); --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -361,7 +361,12 @@ int xenmem_add_to_physmap_one(struct dom unsigned long idx, xen_pfn_t gpfn); /* Returns 0 on success, or negative on error. */ -int guest_remove_page(struct domain *d, unsigned long gmfn); +int __must_check guest_remove_page(struct domain *d, unsigned long gmfn); +/* Remove a page from a domain's p2m table */ +int __must_check +guest_physmap_remove_page(struct domain *d, unsigned long gfn, + unsigned long mfn, unsigned int page_order); + #define RAM_TYPE_CONVENTIONAL 0x00000001 #define RAM_TYPE_RESERVED 0x00000002
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
