From: Jan Beulich Subject: gnttab: cope with version changes racing other operations Dropping and re-acquiring the grant table lock for a particular operation requires special care, as in the meantime the grant table version can change. During a v2 -> v1 change, status frames going away means that pre- calculated status pointers go stale, referencing freed (and possibly already re-used) memory. Record in-flight v2 operations, permitting the version change only when there are none of them. Recalculate "status" in the one place (map_grant_ref()'s error path) where it could be stale, but confine this to reserved entries. This is CVE-2026-62436. Reported-by: Mark Esler During a v1 -> v2 change, the number of shared table entries reduces, meaning that previously validated grant references may now be out of bounds. Because of the checking of pin counts in gnttab_set_version() (with the grant table lock held for writing), for now-out-of-bounds gref-s neither active mappings can exist, nor can there be in-progress copy operations. Nevertheless bounds checks are added there, just to be on the safe side. For gnttab_transfer(), to cover the gap between the lock being dropped by gnttab_prepare_for_transfer() and it being re-acquired, have the helper return the version it found, and fail the operation if the version turns out to have changed after re-acquiring the lock. Further avoid needless use of shared_entry_header(), as it involves pointer arithmetic which, when using an out-of-bounds ref, is UB. This is CVE-2026-62435. Everything together is XSA-501. Fixes: a98dc13703e0 ("Introduce a grant_entry_v2 structure") Signed-off-by: Jan Beulich Tested-by: Mark Esler Reviewed-by: Andrew Cooper --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -71,6 +71,10 @@ struct grant_table { unsigned int nr_grant_frames; /* Number of grant status frames shared with guest (for version 2) */ unsigned int nr_status_frames; + + /* Number of version 2 operations in progress. */ + atomic_t nr_v2_ops; + /* * Number of available maptrack entries. For cleanup purposes it is * important to realize that this field and @maptrack further down will @@ -933,6 +937,9 @@ static void reduce_status_for_pin(struct { unsigned int clear_flags = act->pin ? 0 : GTF_reading; + if ( unlikely(!status) ) + return; + if ( !readonly && !(act->pin & (GNTPIN_hstw_mask | GNTPIN_devw_mask)) ) clear_flags |= GTF_writing; @@ -1341,6 +1348,22 @@ map_grant_ref( grant_read_lock(rgt); + if ( unlikely(evaluate_nospec((rgt->gt_version == 1) != + (status == &shah->flags))) ) + { + /* + * After a v1 -> v2 change behind our backs "ref" may now be out of + * bounds. Recalculate it, but only for reserved entries. Others + * will have been cleared anyway by the version change. + */ + if ( ref < GNTTAB_NR_RESERVED_ENTRIES ) + status = evaluate_nospec(rgt->gt_version == 1) + ? &shah->flags + : &status_entry(rgt, ref); + else + status = NULL; + } + act = active_entry_acquire(rgt, op->ref); act->pin -= pin_incr; @@ -1584,9 +1607,8 @@ unmap_common_complete(struct gnttab_unma struct domain *ld, *rd = op->rd; struct grant_table *rgt; struct active_grant_entry *act; - grant_entry_header_t *sha; struct page_info *pg; - uint16_t *status; + uint16_t *status = NULL; if ( evaluate_nospec(!op->done) ) { @@ -1602,11 +1624,10 @@ unmap_common_complete(struct gnttab_unma grant_read_lock(rgt); act = active_entry_acquire(rgt, op->ref); - sha = shared_entry_header(rgt, op->ref); if ( evaluate_nospec(rgt->gt_version == 1) ) - status = &sha->flags; - else + status = &shared_entry_v1(rgt, op->ref).flags; + else if ( evaluate_nospec(op->ref < nr_grant_entries(rgt)) ) status = &status_entry(rgt, op->ref); pg = !is_iomem_page(act->mfn) ? mfn_to_page(op->mfn) : NULL; @@ -2194,14 +2215,14 @@ gnttab_query_size( * Check that the given grant reference (rd,ref) allows 'ld' to transfer * ownership of a page frame. If so, lock down the grant entry. */ -static int +static unsigned int gnttab_prepare_for_transfer( struct domain *rd, struct domain *ld, grant_ref_t ref) { struct grant_table *rgt = rd->grant_table; uint32_t *raw_shah; union grant_combo scombo; - int retries = 0; + unsigned int retries = 0, ver; grant_read_lock(rgt); @@ -2246,8 +2267,11 @@ gnttab_prepare_for_transfer( scombo = prev; } + ver = rgt->gt_version; + grant_read_unlock(rgt); - return 1; + + return ver; fail: grant_read_unlock(rgt); @@ -2272,7 +2296,7 @@ gnttab_transfer( for ( i = 0; i < count; i++ ) { - bool_t okay; + unsigned int ver; int rc; if ( i && hypercall_preempt_check() ) @@ -2412,14 +2436,14 @@ gnttab_transfer( * pagelist. */ spin_unlock(&e->page_alloc_lock); - okay = gnttab_prepare_for_transfer(e, d, gop.ref); + ver = gnttab_prepare_for_transfer(e, d, gop.ref); /* * Make sure the reference bound check in gnttab_prepare_for_transfer * is respected and speculative execution is blocked accordingly */ - if ( unlikely(!evaluate_nospec(okay)) || - unlikely(assign_pages(page, 1, e, MEMF_no_refcount)) ) + if ( unlikely(!evaluate_nospec(ver)) || + unlikely(assign_pages(page, 1, e, MEMF_no_refcount)) ) { bool drop_dom_ref; @@ -2431,7 +2455,7 @@ gnttab_transfer( drop_dom_ref = !domain_adjust_tot_pages(e, -1); spin_unlock(&e->page_alloc_lock); - if ( okay /* i.e. e->is_dying due to the surrounding if() */ ) + if ( ver /* i.e. e->is_dying due to the surrounding if() */ ) gdprintk(XENLOG_INFO, "Transferee d%d is now dying\n", e->domain_id); @@ -2451,7 +2475,13 @@ gnttab_transfer( grant_read_lock(e->grant_table); act = active_entry_acquire(e->grant_table, gop.ref); - if ( evaluate_nospec(e->grant_table->gt_version == 1) ) + if ( unlikely(evaluate_nospec(e->grant_table->gt_version != ver)) ) + { + rc = -EILSEQ; + goto release; + } + + if ( evaluate_nospec(ver == 1) ) { grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref); @@ -2471,6 +2501,7 @@ gnttab_transfer( shared_entry_header(e->grant_table, gop.ref)->flags |= GTF_transfer_completed; + release: active_entry_release(act); grant_read_unlock(e->grant_table); @@ -2499,28 +2530,27 @@ release_grant_for_copy( struct domain *rd, grant_ref_t gref, bool readonly) { struct grant_table *rgt = rd->grant_table; - grant_entry_header_t *sha; struct active_grant_entry *act; mfn_t mfn; - uint16_t *status; + uint16_t *status = NULL; grant_ref_t trans_gref; struct domain *td; grant_read_lock(rgt); act = active_entry_acquire(rgt, gref); - sha = shared_entry_header(rgt, gref); mfn = act->mfn; if ( evaluate_nospec(rgt->gt_version == 1) ) { - status = &sha->flags; + status = &shared_entry_v1(rgt, gref).flags; td = rd; trans_gref = gref; } else { - status = &status_entry(rgt, gref); + if ( evaluate_nospec(gref < nr_grant_entries(rgt)) ) + status = &status_entry(rgt, gref); td = (act->src_domid == rd->domain_id) ? rd : knownalive_domain_from_domid(act->src_domid); trans_gref = act->trans_gref; @@ -2539,6 +2569,9 @@ release_grant_for_copy( reduce_status_for_pin(rd, act, status, readonly); + if ( !act->pin && act->is_sub_page ) + atomic_dec(&rgt->nr_v2_ops); + active_entry_release(act); grant_read_unlock(rgt); @@ -2670,8 +2703,10 @@ acquire_grant_for_copy( /* * acquire_grant_for_copy() will take the lock on the remote table, - * so we have to drop the lock here and reacquire. + * so we have to drop the lock here and reacquire. Before doing so, + * record that a v2 operation is in progress. */ + atomic_inc(&rgt->nr_v2_ops); active_entry_release(act); grant_read_unlock(rgt); @@ -2685,6 +2720,7 @@ acquire_grant_for_copy( if ( rc != GNTST_okay ) { + atomic_dec(&rgt->nr_v2_ops); rcu_unlock_domain(td); reduce_status_for_pin(rd, act, status, readonly); active_entry_release(act); @@ -2721,6 +2757,8 @@ acquire_grant_for_copy( rcu_unlock_domain(td); grant_read_lock(rgt); + atomic_dec(&rgt->nr_v2_ops); + act = active_entry_acquire(rgt, gref); reduce_status_for_pin(rd, act, status, readonly); active_entry_release(act); @@ -2747,6 +2785,8 @@ acquire_grant_for_copy( */ act->is_sub_page = true; } + else + atomic_dec(&rgt->nr_v2_ops); } else if ( !old_pin || (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) ) @@ -2801,6 +2841,9 @@ acquire_grant_for_copy( act->src_domid = td->domain_id; act->trans_gref = trans_gref; act->mfn = grant_mfn; + + if ( is_sub_page ) + atomic_inc(&rgt->nr_v2_ops); } else if ( !mfn_eq(act->mfn, grant_mfn) || act->src_domid != td->domain_id || @@ -3226,7 +3269,17 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA if ( res < 0) goto out_unlock; break; + case 2: + if ( atomic_read(>->nr_v2_ops) ) + { + gdprintk(XENLOG_WARNING, + "tried to change to grant table v1, but %d v2 operations still in progress\n", + atomic_read(>->nr_v2_ops)); + res = -EAGAIN; + goto out_unlock; + } + for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ ) { switch ( shared_entry_v2(gt, i).hdr.flags & GTF_type_mask )