|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.7] guest_physmap_remove_page() needs its return value checked
commit eeba17403e1bc33453ce49c5ded951f115ebf1e5
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Jun 20 16:18:43 2017 +0200
Commit: Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Jun 20 16:18:43 2017 +0200
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@xxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
master commit: a0cce6048d010a30ac82f8db7787bbf9aada64f4
master date: 2017-06-20 14:41:16 +0200
---
xen/arch/arm/mm.c | 5 +++--
xen/arch/arm/p2m.c | 15 +++++++--------
xen/arch/x86/domain.c | 10 +++++++++-
xen/arch/x86/domain_build.c | 4 +++-
xen/arch/x86/hvm/ioreq.c | 5 +++--
xen/arch/x86/mm.c | 21 +++++++++++++++------
xen/arch/x86/mm/p2m.c | 7 +++++--
xen/common/grant_table.c | 32 +++++++++++++++-----------------
xen/common/memory.c | 21 +++++++++++++--------
xen/drivers/passthrough/arm/smmu.c | 4 +---
xen/include/asm-arm/p2m.h | 4 ----
xen/include/asm-x86/p2m.h | 5 -----
xen/include/xen/mm.h | 2 +-
xen/include/xen/p2m-common.h | 6 ++++++
14 files changed, 81 insertions(+), 60 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 38bdeac..8df03b9 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1309,13 +1309,14 @@ int replace_grant_host_mapping(unsigned long addr,
unsigned long mfn,
{
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)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 9162f5b..3a749dd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1313,15 +1313,14 @@ int guest_physmap_add_entry(struct domain *d,
d->arch.p2m.default_access);
}
-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, 0, p2m_invalid,
- d->arch.p2m.default_access);
+ return apply_p2m_changes(d, REMOVE,
+ pfn_to_paddr(gfn),
+ pfn_to_paddr(gfn + (1 << page_order)),
+ pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
+ d->arch.p2m.default_access);
}
int p2m_alloc_table(struct domain *d)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ddb4197..452748d 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -802,7 +802,15 @@ int arch_domain_soft_reset(struct domain *d)
ret = -ENOMEM;
goto exit_put_gfn;
}
- guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+
+ ret = guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+ if ( ret )
+ {
+ printk(XENLOG_G_ERR "Failed to remove Dom%d's shared_info frame %lx\n",
+ d->domain_id, gfn);
+ free_domheap_page(new_page);
+ goto exit_put_gfn;
+ }
ret = guest_physmap_add_page(d, gfn, page_to_mfn(new_page), PAGE_ORDER_4K);
if ( ret )
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index f9a3eca..1661ad4 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -427,7 +427,9 @@ static __init void pvh_add_mem_mapping(struct domain *d,
unsigned long gfn,
if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
{
omfn = get_gfn_query_unlocked(d, gfn + i, &t);
- guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
+ if ( guest_physmap_remove_page(d, gfn + i, mfn_x(omfn),
+ PAGE_ORDER_4K) )
+ /* nothing, best effort only */;
continue;
}
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 333ce14..46b9386 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -267,8 +267,9 @@ bool_t is_ioreq_server_page(struct domain *d, const struct
page_info *page)
static void hvm_remove_ioreq_gmfn(
struct domain *d, struct hvm_ioreq_page *iorp)
{
- guest_physmap_remove_page(d, iorp->gmfn,
- page_to_mfn(iorp->page), 0);
+ if ( guest_physmap_remove_page(d, iorp->gmfn,
+ page_to_mfn(iorp->page), 0) )
+ domain_crash(d);
clear_page(iorp->va);
}
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 00818e1..0426b6e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4271,7 +4271,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;
@@ -4796,7 +4800,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 )
@@ -4870,25 +4874,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);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 5b8b7b9..7bbb782 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2840,10 +2840,12 @@ int p2m_add_foreign(struct domain *tdom, unsigned long
fgfn,
{
if ( is_xen_heap_mfn(prev_mfn) )
/* Xen heap frames are simply unhooked from this phys slot */
- guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
+ rc = guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
else
/* Normal domain memory is freed, to avoid leaking memory. */
- guest_remove_page(tdom, gpfn);
+ rc = guest_remove_page(tdom, gpfn);
+ if ( rc )
+ goto put_both;
}
/*
* Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2856,6 +2858,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long
fgfn,
"gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id);
+ put_both:
put_page(page);
/*
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index bd0880f..396c99d 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1804,6 +1804,7 @@ gnttab_transfer(
for ( i = 0; i < count; i++ )
{
bool_t okay;
+ int rc;
if (i && hypercall_preempt_check())
return i;
@@ -1854,27 +1855,33 @@ gnttab_transfer(
goto copyback;
}
- guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+ rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0);
gnttab_flush_tlb(d);
+ 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;
@@ -1923,12 +1930,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'. */
@@ -1957,13 +1960,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);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 8986cad..148b8fe 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -250,8 +250,12 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
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 )
@@ -315,7 +319,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
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);
/*
@@ -326,16 +332,14 @@ int guest_remove_page(struct domain *d, unsigned long
gmfn)
* For this purpose (and to match populate_physmap() behavior), the page
* is kept allocated.
*/
- if ( !is_domain_direct_mapped(d) &&
+ if ( !rc && !is_domain_direct_mapped(d) &&
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)
@@ -570,7 +574,8 @@ static long
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
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);
}
@@ -1120,7 +1125,7 @@ long do_memory_op(unsigned long cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
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
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index 54a03a6..9c835b2 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2783,9 +2783,7 @@ static int arm_smmu_unmap_page(struct domain *d, unsigned
long gfn)
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- guest_physmap_remove_page(d, gfn, gfn, 0);
-
- return 0;
+ return guest_physmap_remove_page(d, gfn, gfn, 0);
}
static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index cb19350..0427fa4 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -177,10 +177,6 @@ static inline int guest_physmap_add_page(struct domain *d,
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);
/*
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 65675a2..e03d449 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -558,11 +558,6 @@ static inline int guest_physmap_add_page(struct domain *d,
return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
}
-/* Remove a page from a domain's p2m table */
-int 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);
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 193b993..4b3b013 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -510,7 +510,7 @@ int xenmem_add_to_physmap_one(struct domain *d, unsigned
int space,
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);
#define RAM_TYPE_CONVENTIONAL 0x00000001
#define RAM_TYPE_RESERVED 0x00000002
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
index 6374a5b..cd41690 100644
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,7 @@
#ifndef _XEN_P2M_COMMON_H
#define _XEN_P2M_COMMON_H
+#include <xen/mm.h>
#include <public/vm_event.h>
/*
@@ -33,6 +34,11 @@ typedef enum {
/* NOTE: Assumed to be only 4 bits right now on x86. */
} p2m_access_t;
+/* 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);
+
/* Map MMIO regions in the p2m: start_gfn and nr describe the range in
* * the guest physical address space to map, starting from the machine
* * frame number mfn. */
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.7
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |