Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP1:GA
xen.10697
59958ebf-gnttab-fix-transitive-grant-handling.p...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 59958ebf-gnttab-fix-transitive-grant-handling.patch of Package xen.10697
# Commit ad48fb963dbff02762d2db5396fa655ac0c432c7 # Date 2017-08-17 14:40:31 +0200 # Author Jan Beulich <jbeulich@suse.com> # Committer Jan Beulich <jbeulich@suse.com> 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@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -1752,13 +1752,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); @@ -1788,30 +1783,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); } @@ -1892,79 +1878,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 ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive ) { - if ( (rc = _set_status(rgt->gt_version, ldom, - readonly, 0, shah, act, - status) ) != GNTST_okay ) - goto unlock_out; + 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 ) @@ -2239,14 +2259,35 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA } } - /* 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) */
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
