|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.5] gnttab: fix transitive grant handling
commit 136ff4ea88123d7728a01187ee9bbdf010b23345
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Thu Aug 17 15:15:55 2017 +0200
Commit: Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Aug 17 15:15:55 2017 +0200
gnttab: fix transitive grant handling
Processing of transitive grants must not use the fast path, or else
reference counting breaks due to the skipped recursive call to
__acquire_grant_for_copy() (its __release_grant_for_copy()
counterpart occurs independent of original pin count). Furthermore
after re-acquiring temporarily dropped locks we need to verify no grant
properties changed if the original pin count was non-zero; checking
just the pin counts is sufficient only for well-behaved guests. As a
result, __release_grant_for_copy() needs to mirror that new behavior.
Furthermore a __release_grant_for_copy() invocation was missing on the
retry path of __acquire_grant_for_copy(), and gnttab_set_version() also
needs to bail out upon encountering a transitive grant.
This is part of XSA-226.
Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
master commit: ad48fb963dbff02762d2db5396fa655ac0c432c7
master date: 2017-08-17 14:40:31 +0200
---
xen/common/grant_table.c | 217 ++++++++++++++++++++++++++++-------------------
1 file changed, 129 insertions(+), 88 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index e6b9073..f191ed4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1808,13 +1808,8 @@ __release_grant_for_copy(
unsigned long r_frame;
uint16_t *status;
grant_ref_t trans_gref;
- int released_read;
- int released_write;
struct domain *td;
- released_read = 0;
- released_write = 0;
-
spin_lock(&rgt->lock);
act = &active_entry(rgt, gref);
@@ -1844,30 +1839,21 @@ __release_grant_for_copy(
act->pin -= GNTPIN_hstw_inc;
if ( !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) )
- {
- released_write = 1;
gnttab_clear_flag(_GTF_writing, status);
- }
}
if ( !act->pin )
- {
gnttab_clear_flag(_GTF_reading, status);
- released_read = 1;
- }
spin_unlock(&rgt->lock);
if ( td != rd )
{
/*
- * Recursive calls, but they're bounded (acquire permits only a single
+ * Recursive call, but it is bounded (acquire permits only a single
* level of transitivity), so it's okay.
*/
- if ( released_write )
- __release_grant_for_copy(td, trans_gref, 0);
- else if ( released_read )
- __release_grant_for_copy(td, trans_gref, 1);
+ __release_grant_for_copy(td, trans_gref, readonly);
rcu_unlock_domain(td);
}
@@ -1948,79 +1934,113 @@ __acquire_grant_for_copy(
act->domid, ldom, act->pin);
old_pin = act->pin;
- if ( !act->pin ||
- (!readonly && !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
- {
- if ( (rc = _set_status(rgt->gt_version, ldom,
- readonly, 0, shah, act,
- status) ) != GNTST_okay )
- goto unlock_out;
+ if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
+ {
+ if ( (!old_pin || (!readonly &&
+ !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))))
&&
+ (rc = _set_status_v2(ldom, readonly, 0, shah, act,
+ status)) != GNTST_okay )
+ goto unlock_out;
+
+ if ( !allow_transitive )
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grant when transitivity not allowed\n");
+
+ trans_domid = sha2->transitive.trans_domid;
+ trans_gref = sha2->transitive.gref;
+ barrier(); /* Stop the compiler from re-loading
+ trans_domid from shared memory */
+ if ( trans_domid == rd->domain_id )
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grants cannot be self-referential\n");
- td = rd;
- trans_gref = gref;
- if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
+ /*
+ * We allow the trans_domid == ldom case, which corresponds to a
+ * grant being issued by one domain, sent to another one, and then
+ * transitively granted back to the original domain. Allowing it
+ * is easy, and means that you don't need to go out of your way to
+ * avoid it in the guest.
+ */
+
+ /* We need to leave the rrd locked during the grant copy. */
+ td = rcu_lock_domain_by_id(trans_domid);
+ if ( td == NULL )
+ PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grant referenced bad domain %d\n",
+ trans_domid);
+
+ /*
+ * __acquire_grant_for_copy() could take the lock on the
+ * remote table (if rd == td), so we have to drop the lock
+ * here and reacquire.
+ */
+ spin_unlock(&rgt->lock);
+
+ rc = __acquire_grant_for_copy(td, trans_gref, rd->domain_id,
+ readonly, &grant_frame, page,
+ &trans_page_off, &trans_length, 0);
+
+ spin_lock(&rgt->lock);
+
+ if ( rc != GNTST_okay )
{
- if ( !allow_transitive )
- PIN_FAIL(unlock_out_clear, GNTST_general_error,
- "transitive grant when transitivity not allowed\n");
-
- trans_domid = sha2->transitive.trans_domid;
- trans_gref = sha2->transitive.gref;
- barrier(); /* Stop the compiler from re-loading
- trans_domid from shared memory */
- if ( trans_domid == rd->domain_id )
- PIN_FAIL(unlock_out_clear, GNTST_general_error,
- "transitive grants cannot be self-referential\n");
-
- /* We allow the trans_domid == ldom case, which
- corresponds to a grant being issued by one domain, sent
- to another one, and then transitively granted back to
- the original domain. Allowing it is easy, and means
- that you don't need to go out of your way to avoid it
- in the guest. */
-
- /* We need to leave the rrd locked during the grant copy */
- td = rcu_lock_domain_by_id(trans_domid);
- if ( td == NULL )
- PIN_FAIL(unlock_out_clear, GNTST_general_error,
- "transitive grant referenced bad domain %d\n",
- trans_domid);
+ __fixup_status_for_copy_pin(act, status);
+ rcu_unlock_domain(td);
spin_unlock(&rgt->lock);
+ return rc;
+ }
- rc = __acquire_grant_for_copy(td, trans_gref, rd->domain_id,
- readonly, &grant_frame, page,
- &trans_page_off, &trans_length, 0);
-
- spin_lock(&rgt->lock);
- if ( rc != GNTST_okay ) {
- __fixup_status_for_copy_pin(act, status);
- rcu_unlock_domain(td);
- spin_unlock(&rgt->lock);
- return rc;
- }
+ /*
+ * We dropped the lock, so we have to check that the grant didn't
+ * change, and that nobody else tried to pin/unpin it. If anything
+ * changed, just give up and tell the caller to retry.
+ */
+ if ( rgt->gt_version != 2 ||
+ act->pin != old_pin ||
+ (old_pin && (act->domid != ldom || act->frame != grant_frame ||
+ act->start != trans_page_off ||
+ act->length != trans_length ||
+ act->trans_domain != td ||
+ act->trans_gref != trans_gref ||
+ !act->is_sub_page)) )
+ {
+ __release_grant_for_copy(td, trans_gref, readonly);
+ __fixup_status_for_copy_pin(act, status);
+ rcu_unlock_domain(td);
+ spin_unlock(&rgt->lock);
+ put_page(*page);
+ *page = NULL;
+ return ERESTART;
+ }
+ if ( !old_pin )
+ {
+ act->domid = ldom;
+ act->start = trans_page_off;
+ act->length = trans_length;
+ act->trans_domain = td;
+ act->trans_gref = trans_gref;
+ act->frame = grant_frame;
+ act->gfn = -1ul;
/*
- * We dropped the lock, so we have to check that nobody else tried
- * to pin (or, for that matter, unpin) the reference in *this*
- * domain. If they did, just give up and tell the caller to retry.
+ * The actual remote remote grant may or may not be a sub-page,
+ * but we always treat it as one because that blocks mappings of
+ * transitive grants.
*/
- if ( act->pin != old_pin )
- {
- __fixup_status_for_copy_pin(act, status);
- rcu_unlock_domain(td);
- spin_unlock(&rgt->lock);
- put_page(*page);
- *page = NULL;
- return ERESTART;
- }
-
- /* The actual remote remote grant may or may not be a
- sub-page, but we always treat it as one because that
- blocks mappings of transitive grants. */
- is_sub_page = 1;
- act->gfn = -1ul;
+ act->is_sub_page = 1;
}
- else if ( sha1 )
+ }
+ else if ( !old_pin ||
+ (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
+ {
+ if ( (rc = _set_status(rgt->gt_version, ldom,
+ readonly, 0, shah, act,
+ status) ) != GNTST_okay )
+ goto unlock_out;
+
+ td = rd;
+ trans_gref = gref;
+ if ( sha1 )
{
rc = __get_paged_frame(sha1->frame, &grant_frame, page, readonly,
rd);
if ( rc != GNTST_okay )
@@ -2295,14 +2315,35 @@
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
}
}
- /* XXX: If we're going to version 2, we could maybe shrink the
- active grant table here. */
-
- if ( op.version == 2 && gt->gt_version < 2 )
+ switch ( gt->gt_version )
{
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
- if ( res < 0)
- goto out_unlock;
+ case 0:
+ if ( op.version == 2 )
+ {
+ case 1:
+ /* XXX: We could maybe shrink the active grant table here. */
+ res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ if ( res < 0)
+ goto out_unlock;
+ }
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ switch ( shared_entry_v2(gt, i).hdr.flags & GTF_type_mask )
+ {
+ case GTF_permit_access:
+ if ( !(shared_entry_v2(gt, i).full_page.frame >> 32) )
+ break;
+ /* fall through */
+ case GTF_transitive:
+ gdprintk(XENLOG_WARNING,
+ "tried to change grant table version to 1 with
non-representable entries\n");
+ res = -ERANGE;
+ goto out_unlock;
+ }
+ }
+ break;
}
/* Preserve the first 8 entries (toolstack reserved grants) */
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.5
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |