|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen staging-4.17] gnttab: cope with version changes racing other operations
commit 1cc5e5babc1ab6f454d0a970ab52270c20402dc7
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Jul 28 12:55:54 2026 +0100
Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Jul 28 13:10:38 2026 +0100
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 <mark@xxxxxxxxxxxx>
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 <jbeulich@xxxxxxxx>
Tested-by: Mark Esler <mark@xxxxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
(cherry picked from commit 96dfeb41774ab50c74a01483726db8bb17a31a69)
---
xen/common/grant_table.c | 93 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 73 insertions(+), 20 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index bf1b0d3677..b65979791c 100644
--- 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
@@ -918,6 +922,9 @@ static void reduce_status_for_pin(struct domain *rd,
{
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;
@@ -1318,6 +1325,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;
@@ -1557,9 +1580,8 @@ unmap_common_complete(struct gnttab_unmap_common *op)
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) )
{
@@ -1575,11 +1597,10 @@ unmap_common_complete(struct gnttab_unmap_common *op)
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;
@@ -2167,14 +2188,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);
@@ -2219,8 +2240,11 @@ gnttab_prepare_for_transfer(
scombo = prev;
}
+ ver = rgt->gt_version;
+
grant_read_unlock(rgt);
- return 1;
+
+ return ver;
fail:
grant_read_unlock(rgt);
@@ -2245,7 +2269,7 @@ gnttab_transfer(
for ( i = 0; i < count; i++ )
{
- bool_t okay;
+ unsigned int ver;
int rc;
if ( i && hypercall_preempt_check() )
@@ -2385,14 +2409,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;
@@ -2404,7 +2428,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);
@@ -2424,7 +2448,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);
@@ -2444,6 +2474,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);
@@ -2472,28 +2503,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->trans_domain;
trans_gref = act->trans_gref;
}
@@ -2511,6 +2541,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);
@@ -2623,8 +2656,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);
@@ -2638,6 +2673,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);
@@ -2674,6 +2710,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);
@@ -2700,6 +2738,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))) )
@@ -2754,6 +2794,9 @@ acquire_grant_for_copy(
act->trans_domain = td;
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->trans_domain != td ||
@@ -3173,7 +3216,17 @@
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
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 )
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.17
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |