|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] x86/mm: Fix liveness of pages in grant copy operations
# HG changeset patch
# User Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
# Date 1322154325 0
# Node ID 8fb3fa6218c600393c946ab85c7635299208a66d
# Parent f011ebdce88bf95c5021777d4377bb016a2f0275
x86/mm: Fix liveness of pages in grant copy operations
We were immediately putting the p2m entry translation for grant
copy operations. This allowed for an unnecessary race by which the
page could have been swapped out between the p2m lookup and the actual
use. Hold on to the p2m entries until the grant operation finishes.
Also fixes a small bug: for the source page of the copy, get_page
was assuming the page was owned by the source domain. It may be a
shared page, since we don't perform an unsharing p2m lookup.
Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
Acked-by: Tim Deegan <tim@xxxxxxx>
Committed-by: Tim Deegan <tim@xxxxxxx>
---
diff -r f011ebdce88b -r 8fb3fa6218c6 xen/common/grant_table.c
--- a/xen/common/grant_table.c Thu Nov 24 17:05:25 2011 +0000
+++ b/xen/common/grant_table.c Thu Nov 24 17:05:25 2011 +0000
@@ -1723,11 +1723,12 @@
/* Grab a frame number from a grant entry and update the flags and pin
count as appropriate. Note that this does *not* update the page
type or reference counts, and does not check that the mfn is
- actually valid. */
+ actually valid. If *gfn != INVALID_GFN, and rc == GNTST_okay, then
+ we leave this function holding the p2m entry for *gfn in *owning_domain */
static int
__acquire_grant_for_copy(
struct domain *rd, unsigned long gref, struct domain *ld, int readonly,
- unsigned long *frame, unsigned *page_off, unsigned *length,
+ unsigned long *frame, unsigned long *gfn, unsigned *page_off, unsigned
*length,
unsigned allow_transitive, struct domain **owning_domain)
{
grant_entry_v1_t *sha1;
@@ -1739,7 +1740,6 @@
domid_t trans_domid;
grant_ref_t trans_gref;
struct domain *td;
- unsigned long gfn;
unsigned long grant_frame;
unsigned trans_page_off;
unsigned trans_length;
@@ -1748,6 +1748,7 @@
s16 rc = GNTST_okay;
*owning_domain = NULL;
+ *gfn = INVALID_GFN;
spin_lock(&rd->grant_table->lock);
@@ -1824,7 +1825,7 @@
spin_unlock(&rd->grant_table->lock);
rc = __acquire_grant_for_copy(td, trans_gref, rd,
- readonly, &grant_frame,
+ readonly, &grant_frame, gfn,
&trans_page_off, &trans_length,
0, &ignore);
@@ -1846,7 +1847,7 @@
rcu_unlock_domain(td);
spin_unlock(&rd->grant_table->lock);
return __acquire_grant_for_copy(rd, gref, ld, readonly,
- frame, page_off, length,
+ frame, gfn, page_off, length,
allow_transitive,
owning_domain);
}
@@ -1860,13 +1861,11 @@
}
else if ( sha1 )
{
- gfn = sha1->frame;
- rc = __get_paged_frame(gfn, &grant_frame, readonly, rd);
- /* We drop this immediately per the comments at the top */
- put_gfn(rd, gfn);
+ *gfn = sha1->frame;
+ rc = __get_paged_frame(*gfn, &grant_frame, readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out;
- act->gfn = gfn;
+ act->gfn = *gfn;
is_sub_page = 0;
trans_page_off = 0;
trans_length = PAGE_SIZE;
@@ -1874,12 +1873,11 @@
}
else if ( !(sha2->hdr.flags & GTF_sub_page) )
{
- gfn = sha2->full_page.frame;
- rc = __get_paged_frame(gfn, &grant_frame, readonly, rd);
- put_gfn(rd, gfn);
+ *gfn = sha2->full_page.frame;
+ rc = __get_paged_frame(*gfn, &grant_frame, readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out;
- act->gfn = gfn;
+ act->gfn = *gfn;
is_sub_page = 0;
trans_page_off = 0;
trans_length = PAGE_SIZE;
@@ -1887,12 +1885,11 @@
}
else
{
- gfn = sha2->sub_page.frame;
- rc = __get_paged_frame(gfn, &grant_frame, readonly, rd);
- put_gfn(rd, gfn);
+ *gfn = sha2->sub_page.frame;
+ rc = __get_paged_frame(*gfn, &grant_frame, readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out;
- act->gfn = gfn;
+ act->gfn = *gfn;
is_sub_page = 1;
trans_page_off = sha2->sub_page.page_off;
trans_length = sha2->sub_page.length;
@@ -1932,7 +1929,7 @@
{
struct domain *sd = NULL, *dd = NULL;
struct domain *source_domain = NULL, *dest_domain = NULL;
- unsigned long s_frame, d_frame;
+ unsigned long s_frame, d_frame, s_gfn = INVALID_GFN, d_gfn = INVALID_GFN;
char *sp, *dp;
s16 rc = GNTST_okay;
int have_d_grant = 0, have_s_grant = 0, have_s_ref = 0;
@@ -1973,14 +1970,14 @@
{
unsigned source_off, source_len;
rc = __acquire_grant_for_copy(sd, op->source.u.ref, current->domain, 1,
- &s_frame, &source_off, &source_len, 1,
+ &s_frame, &s_gfn, &source_off,
&source_len, 1,
&source_domain);
if ( rc != GNTST_okay )
goto error_out;
have_s_grant = 1;
if ( op->source.offset < source_off ||
op->len > source_len )
- PIN_FAIL(error_out, GNTST_general_error,
+ PIN_FAIL(error_put_s_gfn, GNTST_general_error,
"copy source out of bounds: %d < %d || %d > %d\n",
op->source.offset, source_off,
op->len, source_len);
@@ -1988,8 +1985,8 @@
else
{
#ifdef CONFIG_X86
+ s_gfn = op->source.u.gmfn;
rc = __get_paged_frame(op->source.u.gmfn, &s_frame, 1, sd);
- put_gfn(sd, op->source.u.gmfn);
if ( rc != GNTST_okay )
goto error_out;
#else
@@ -1998,14 +1995,16 @@
source_domain = sd;
}
if ( unlikely(!mfn_valid(s_frame)) )
- PIN_FAIL(error_out, GNTST_general_error,
+ PIN_FAIL(error_put_s_gfn, GNTST_general_error,
"source frame %lx invalid.\n", s_frame);
- if ( !get_page(mfn_to_page(s_frame), source_domain) )
+ /* For the source frame, the page could still be shared, so
+ * don't assume ownership by source_domain */
+ if ( !page_get_owner_and_reference(mfn_to_page(s_frame)) )
{
if ( !sd->is_dying )
gdprintk(XENLOG_WARNING, "Could not get src frame %lx\n", s_frame);
rc = GNTST_general_error;
- goto error_out;
+ goto error_put_s_gfn;
}
have_s_ref = 1;
@@ -2013,14 +2012,14 @@
{
unsigned dest_off, dest_len;
rc = __acquire_grant_for_copy(dd, op->dest.u.ref, current->domain, 0,
- &d_frame, &dest_off, &dest_len, 1,
+ &d_frame, &d_gfn, &dest_off, &dest_len,
1,
&dest_domain);
if ( rc != GNTST_okay )
- goto error_out;
+ goto error_put_s_gfn;
have_d_grant = 1;
if ( op->dest.offset < dest_off ||
op->len > dest_len )
- PIN_FAIL(error_out, GNTST_general_error,
+ PIN_FAIL(error_put_d_gfn, GNTST_general_error,
"copy dest out of bounds: %d < %d || %d > %d\n",
op->dest.offset, dest_off,
op->len, dest_len);
@@ -2028,17 +2027,17 @@
else
{
#ifdef CONFIG_X86
+ d_gfn = op->dest.u.gmfn;
rc = __get_paged_frame(op->dest.u.gmfn, &d_frame, 0, dd);
- put_gfn(dd, op->dest.u.gmfn);
if ( rc != GNTST_okay )
- goto error_out;
+ goto error_put_s_gfn;
#else
d_frame = gmfn_to_mfn(dd, op->dest.u.gmfn);
#endif
dest_domain = dd;
}
if ( unlikely(!mfn_valid(d_frame)) )
- PIN_FAIL(error_out, GNTST_general_error,
+ PIN_FAIL(error_put_d_gfn, GNTST_general_error,
"destination frame %lx invalid.\n", d_frame);
if ( !get_page_and_type(mfn_to_page(d_frame), dest_domain,
PGT_writable_page) )
@@ -2046,7 +2045,7 @@
if ( !dd->is_dying )
gdprintk(XENLOG_WARNING, "Could not get dst frame %lx\n", d_frame);
rc = GNTST_general_error;
- goto error_out;
+ goto error_put_d_gfn;
}
sp = map_domain_page(s_frame);
@@ -2060,6 +2059,12 @@
gnttab_mark_dirty(dd, d_frame);
put_page_and_type(mfn_to_page(d_frame));
+ error_put_d_gfn:
+ if ( (d_gfn != INVALID_GFN) && (dest_domain) )
+ put_gfn(dest_domain, d_gfn);
+ error_put_s_gfn:
+ if ( (s_gfn != INVALID_GFN) && (source_domain) )
+ put_gfn(source_domain, s_gfn);
error_out:
if ( have_s_ref )
put_page(mfn_to_page(s_frame));
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |