From 29f04a077972e07c86c9e911005220f6d691ffa6 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Thu, 15 Jun 2017 12:05:29 +0100 Subject: [PATCH 3/3] gnttab: correct maptrack table accesses In order to observe a consistent (limit,pointer-table) pair, the reader needs to either hold the maptrack lock (in line with documentation) or both sides need to order their accesses suitably (the writer side barrier was removed by commit dff515dfea ["gnttab: use per-VCPU maptrack free lists"], and a read side barrier has never been there). Make the writer publish a new table page before limit (for bounds checks to work), and new list head last (for racing maptrack_entry() invocations to work). At the same time add read barriers to lockless readers. Additionally get_maptrack_handle() must not assume ->maptrack_head to not change behind its back: Another handle may be put (updating only ->maptrack_tail) and then got or stolen (updating ->maptrack_head). This is part of XSA-218. Signed-off-by: Jan Beulich Reviewed-by: George Dunlap --- xen/common/grant_table.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index ba10e76..627947a 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -395,7 +395,7 @@ get_maptrack_handle( struct grant_table *lgt) { struct vcpu *curr = current; - int i; + unsigned int i, head; grant_handle_t handle; struct grant_mapping *new_mt; @@ -451,17 +451,20 @@ get_maptrack_handle( new_mt[i].ref = handle + i + 1; new_mt[i].vcpu = curr->vcpu_id; } - new_mt[i - 1].ref = curr->maptrack_head; /* Set tail directly if this is the first page for this VCPU. */ if ( curr->maptrack_tail == MAPTRACK_TAIL ) curr->maptrack_tail = handle + MAPTRACK_PER_PAGE - 1; - write_atomic(&curr->maptrack_head, handle + 1); - lgt->maptrack[nr_maptrack_frames(lgt)] = new_mt; + smp_wmb(); lgt->maptrack_limit += MAPTRACK_PER_PAGE; + do { + new_mt[i - 1].ref = read_atomic(&curr->maptrack_head); + head = cmpxchg(&curr->maptrack_head, new_mt[i - 1].ref, handle + 1); + } while ( head != new_mt[i - 1].ref ); + spin_unlock(&lgt->maptrack_lock); return handle; @@ -727,6 +730,7 @@ static unsigned int mapkind( for ( handle = 0; !(kind & MAPKIND_WRITE) && handle < lgt->maptrack_limit; handle++ ) { + smp_rmb(); map = &maptrack_entry(lgt, handle); if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) || map->domid != rd->domain_id ) @@ -1094,6 +1098,7 @@ __gnttab_unmap_common( return; } + smp_rmb(); map = &maptrack_entry(lgt, op->handle); grant_read_lock(lgt); -- 2.1.4