|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] Be careful with page_get_owner() now that owner field can be clobbered
# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1236176930 0
# Node ID 4b7d638a8b89b6d49094e77d6295c6d8ffafc192
# Parent 7f573cb76db41a9fc46052867b02e9e0c107aa86
Be careful with page_get_owner() now that owner field can be clobbered
by some users. Introduce get_page_owner_and_reference() where that can
be more useful.
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
xen/arch/x86/mm.c | 92 +++++++++++++++++++++++++++++-----------------
xen/common/grant_table.c | 41 ++++++++++++--------
xen/common/xencomm.c | 28 +++++---------
xen/include/asm-ia64/mm.h | 19 ++++++---
xen/include/asm-x86/mm.h | 1
5 files changed, 106 insertions(+), 75 deletions(-)
diff -r 7f573cb76db4 -r 4b7d638a8b89 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Tue Mar 03 13:22:28 2009 +0000
+++ b/xen/arch/x86/mm.c Wed Mar 04 14:28:50 2009 +0000
@@ -371,14 +371,14 @@ void share_xen_page_with_privileged_gues
#else
/*
* In debug builds we shadow a selection of <4GB PDPTs to exercise code paths.
- * We cannot safely shadow the idle page table, nor shadow (v1) page tables
- * (detected by lack of an owning domain). As required for correctness, we
+ * We cannot safely shadow the idle page table, nor shadow page tables
+ * (detected by zero reference count). As required for correctness, we
* always shadow PDPTs above 4GB.
*/
-#define l3tab_needs_shadow(mfn) \
- (((((mfn) << PAGE_SHIFT) != __pa(idle_pg_table)) && \
- (page_get_owner(mfn_to_page(mfn)) != NULL) && \
- ((mfn) & 1)) || /* odd MFNs are shadowed */ \
+#define l3tab_needs_shadow(mfn) \
+ (((((mfn) << PAGE_SHIFT) != __pa(idle_pg_table)) && \
+ (mfn_to_page(mfn)->count_info & PGC_count_mask) && \
+ ((mfn) & 1)) || /* odd MFNs are shadowed */ \
((mfn) >= 0x100000))
#endif
@@ -690,7 +690,16 @@ get_##level##_linear_pagetable(
int is_iomem_page(unsigned long mfn)
{
- return (!mfn_valid(mfn) || (page_get_owner(mfn_to_page(mfn)) == dom_io));
+ struct page_info *page;
+
+ if ( !mfn_valid(mfn) )
+ return 1;
+
+ /* Caller must know that it is an iomem page, or a reference is held. */
+ page = mfn_to_page(mfn);
+ ASSERT((page->count_info & PGC_count_mask) != 0);
+
+ return (page_get_owner(page) == dom_io);
}
@@ -703,7 +712,6 @@ get_page_from_l1e(
uint32_t l1f = l1e_get_flags(l1e);
struct vcpu *curr = current;
struct domain *owner;
- int okay;
if ( !(l1f & _PAGE_PRESENT) )
return 1;
@@ -714,8 +722,13 @@ get_page_from_l1e(
return 0;
}
- if ( is_iomem_page(mfn) )
- {
+ if ( !mfn_valid(mfn) ||
+ (owner = page_get_owner_and_reference(page)) == dom_io )
+ {
+ /* Only needed the reference to confirm dom_io ownership. */
+ if ( mfn_valid(mfn) )
+ put_page(page);
+
/* DOMID_IO reverts to caller for privilege checks. */
if ( d == dom_io )
d = curr->domain;
@@ -730,6 +743,9 @@ get_page_from_l1e(
return 1;
}
+
+ if ( owner == NULL )
+ goto could_not_pin;
/*
* Let privileged domains transfer the right to map their target
@@ -737,27 +753,20 @@ get_page_from_l1e(
* until pvfb supports granted mappings. At that time this minor hack
* can go away.
*/
- owner = page_get_owner(page);
- if ( unlikely(d != owner) && (owner != NULL) &&
- (d != curr->domain) && IS_PRIV_FOR(d, owner) )
+ if ( unlikely(d != owner) && (d != curr->domain) && IS_PRIV_FOR(d, owner) )
d = owner;
/* Foreign mappings into guests in shadow external mode don't
* contribute to writeable mapping refcounts. (This allows the
* qemu-dm helper process in dom0 to map the domain's memory without
* messing up the count of "real" writable mappings.) */
- okay = get_data_page(
- page, d,
- (l1f & _PAGE_RW) && !(paging_mode_external(d) && (d != curr->domain)));
- if ( !okay )
- {
- MEM_LOG("Error getting mfn %lx (pfn %lx) from L1 entry %" PRIpte
- " for dom%d",
- mfn, get_gpfn_from_mfn(mfn),
- l1e_get_intpte(l1e), d->domain_id);
- }
- else if ( pte_flags_to_cacheattr(l1f) !=
- ((page->count_info >> PGC_cacheattr_base) & 7) )
+ if ( (l1f & _PAGE_RW) &&
+ !(paging_mode_external(d) && (d != curr->domain)) &&
+ !get_page_type(page, PGT_writable_page) )
+ goto could_not_pin;
+
+ if ( pte_flags_to_cacheattr(l1f) !=
+ ((page->count_info >> PGC_cacheattr_base) & 7) )
{
unsigned long x, nx, y = page->count_info;
unsigned long cacheattr = pte_flags_to_cacheattr(l1f);
@@ -786,7 +795,16 @@ get_page_from_l1e(
#endif
}
- return okay;
+ return 1;
+
+ could_not_pin:
+ MEM_LOG("Error getting mfn %lx (pfn %lx) from L1 entry %" PRIpte
+ " for dom%d",
+ mfn, get_gpfn_from_mfn(mfn),
+ l1e_get_intpte(l1e), d->domain_id);
+ if ( owner != NULL )
+ put_page(page);
+ return 0;
}
@@ -1174,7 +1192,7 @@ static int create_pae_xen_mappings(struc
for ( i = 0; i < PDPT_L2_ENTRIES; i++ )
{
l2e = l2e_from_page(
- virt_to_page(page_get_owner(page)->arch.mm_perdomain_pt) + i,
+ virt_to_page(d->arch.mm_perdomain_pt) + i,
__PAGE_HYPERVISOR);
l2e_write(&pl2e[l2_table_offset(PERDOMAIN_VIRT_START) + i], l2e);
}
@@ -1924,7 +1942,7 @@ void put_page(struct page_info *page)
}
-int get_page(struct page_info *page, struct domain *domain)
+struct domain *page_get_owner_and_reference(struct page_info *page)
{
unsigned long x, y = page->count_info;
@@ -1933,22 +1951,29 @@ int get_page(struct page_info *page, str
if ( unlikely((x & PGC_count_mask) == 0) || /* Not allocated? */
/* Keep one spare reference to be acquired by get_page_light(). */
unlikely(((x + 2) & PGC_count_mask) <= 1) ) /* Overflow? */
- goto fail;
+ return NULL;
}
while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x );
- if ( likely(page_get_owner(page) == domain) )
+ return page_get_owner(page);
+}
+
+
+int get_page(struct page_info *page, struct domain *domain)
+{
+ struct domain *owner = page_get_owner_and_reference(page);
+
+ if ( likely(owner == domain) )
return 1;
put_page(page);
- fail:
if ( !_shadow_mode_refcounts(domain) && !domain->is_dying )
gdprintk(XENLOG_INFO,
"Error pfn %lx: rd=%p, od=%p, caf=%08lx, taf=%"
PRtype_info "\n",
- page_to_mfn(page), domain, page_get_owner(page),
- y, page->u.inuse.type_info);
+ page_to_mfn(page), domain, owner,
+ page->count_info, page->u.inuse.type_info);
return 0;
}
@@ -1973,7 +1998,6 @@ static void get_page_light(struct page_i
}
while ( unlikely(y != x) );
}
-
static int alloc_page_type(struct page_info *page, unsigned long type,
int preemptible)
diff -r 7f573cb76db4 -r 4b7d638a8b89 xen/common/grant_table.c
--- a/xen/common/grant_table.c Tue Mar 03 13:22:28 2009 +0000
+++ b/xen/common/grant_table.c Wed Mar 04 14:28:50 2009 +0000
@@ -195,7 +195,7 @@ __gnttab_map_grant_ref(
__gnttab_map_grant_ref(
struct gnttab_map_grant_ref *op)
{
- struct domain *ld, *rd;
+ struct domain *ld, *rd, *owner;
struct vcpu *led;
int handle;
unsigned long frame = 0, nr_gets = 0;
@@ -336,8 +336,13 @@ __gnttab_map_grant_ref(
spin_unlock(&rd->grant_table->lock);
- if ( is_iomem_page(frame) )
- {
+ if ( !mfn_valid(frame) ||
+ (owner = page_get_owner_and_reference(mfn_to_page(frame))) == dom_io )
+ {
+ /* Only needed the reference to confirm dom_io ownership. */
+ if ( mfn_valid(frame) )
+ put_page(mfn_to_page(frame));
+
if ( !iomem_access_permitted(rd, frame, frame) )
{
gdprintk(XENLOG_WARNING,
@@ -352,20 +357,11 @@ __gnttab_map_grant_ref(
if ( rc != GNTST_okay )
goto undo_out;
}
- else
- {
- if ( unlikely(!mfn_valid(frame)) ||
- unlikely(!(gnttab_host_mapping_get_page_type(op, ld, rd) ?
- get_page_and_type(mfn_to_page(frame), rd,
- PGT_writable_page) :
- get_page(mfn_to_page(frame), rd))) )
- {
- if ( !rd->is_dying )
- gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n",
- frame);
- rc = GNTST_general_error;
- goto undo_out;
- }
+ else if ( owner == rd )
+ {
+ if ( gnttab_host_mapping_get_page_type(op, ld, rd) &&
+ !get_page_type(mfn_to_page(frame), PGT_writable_page) )
+ goto could_not_pin;
nr_gets++;
if ( op->flags & GNTMAP_host_map )
@@ -382,6 +378,17 @@ __gnttab_map_grant_ref(
get_page_type(mfn_to_page(frame), PGT_writable_page);
}
}
+ }
+ else
+ {
+ could_not_pin:
+ if ( !rd->is_dying )
+ gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n",
+ frame);
+ if ( owner != NULL )
+ put_page(mfn_to_page(frame));
+ rc = GNTST_general_error;
+ goto undo_out;
}
if ( need_iommu(ld) &&
diff -r 7f573cb76db4 -r 4b7d638a8b89 xen/common/xencomm.c
--- a/xen/common/xencomm.c Tue Mar 03 13:22:28 2009 +0000
+++ b/xen/common/xencomm.c Wed Mar 04 14:28:50 2009 +0000
@@ -51,24 +51,16 @@ xencomm_get_page(unsigned long paddr, st
return -EFAULT;
*page = maddr_to_page(maddr);
- if ( get_page(*page, current->domain) == 0 )
- {
- if ( page_get_owner(*page) != current->domain )
- {
- /*
- * This page might be a page granted by another domain, or
- * this page is freed with decrease reservation hypercall at
- * the same time.
- */
- gdprintk(XENLOG_WARNING,
- "bad page is passed. paddr 0x%lx maddr 0x%lx\n",
- paddr, maddr);
- return -EFAULT;
- }
-
- /* Try again. */
- cpu_relax();
- return -EAGAIN;
+ if ( !get_page(*page, current->domain) == 0 )
+ {
+ /*
+ * This page might be a page granted by another domain, or this page
+ * is freed with decrease reservation hypercall at the same time.
+ */
+ gdprintk(XENLOG_WARNING,
+ "bad page is passed. paddr 0x%lx maddr 0x%lx\n",
+ paddr, maddr);
+ return -EFAULT;
}
return 0;
diff -r 7f573cb76db4 -r 4b7d638a8b89 xen/include/asm-ia64/mm.h
--- a/xen/include/asm-ia64/mm.h Tue Mar 03 13:22:28 2009 +0000
+++ b/xen/include/asm-ia64/mm.h Wed Mar 04 14:28:50 2009 +0000
@@ -200,9 +200,7 @@ static inline void put_page(struct page_
free_domheap_page(page);
}
-/* count_info and ownership are checked atomically. */
-static inline int get_page(struct page_info *page,
- struct domain *domain)
+static inline page_get_owner_and_reference(struct page_info *page)
{
unsigned long x, y = page->count_info;
@@ -210,12 +208,21 @@ static inline int get_page(struct page_i
x = y;
if (unlikely((x & PGC_count_mask) == 0) || /* Not allocated? */
unlikely(((x + 1) & PGC_count_mask) == 0) ) {/* Count overflow? */
- goto fail;
+ return NULL;
}
y = cmpxchg_acq(&page->count_info, x, x + 1);
} while (unlikely(y != x));
- if (likely(page_get_owner(page) == domain))
+ return page_get_owner(page);
+}
+
+/* count_info and ownership are checked atomically. */
+static inline int get_page(struct page_info *page,
+ struct domain *domain)
+{
+ struct domain *owner = page_get_owner_and_reference(page);
+
+ if (likely(owner == domain))
return 1;
put_page(page);
@@ -224,7 +231,7 @@ fail:
gdprintk(XENLOG_INFO,
"Error pfn %lx: rd=%p, od=%p, caf=%016lx, taf=%" PRtype_info "\n",
page_to_mfn(page), domain,
- page_get_owner(page), y, page->u.inuse.type_info);
+ owner, page->count_info, page->u.inuse.type_info);
return 0;
}
diff -r 7f573cb76db4 -r 4b7d638a8b89 xen/include/asm-x86/mm.h
--- a/xen/include/asm-x86/mm.h Tue Mar 03 13:22:28 2009 +0000
+++ b/xen/include/asm-x86/mm.h Wed Mar 04 14:28:50 2009 +0000
@@ -258,6 +258,7 @@ void cleanup_page_cacheattr(struct page_
int is_iomem_page(unsigned long mfn);
+struct domain *page_get_owner_and_reference(struct page_info *page);
void put_page(struct page_info *page);
int get_page(struct page_info *page, struct domain *domain);
void put_page_type(struct page_info *page);
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |