diff options
Diffstat (limited to 'system/xen/xsa/xsa218-0002-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch')
| -rw-r--r-- | system/xen/xsa/xsa218-0002-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch | 231 |
1 files changed, 0 insertions, 231 deletions
diff --git a/system/xen/xsa/xsa218-0002-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch b/system/xen/xsa/xsa218-0002-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch deleted file mode 100644 index b07c978b3c..0000000000 --- a/system/xen/xsa/xsa218-0002-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch +++ /dev/null @@ -1,231 +0,0 @@ -From bb6d476b09e635baf5e9fb22540ab7c3530d1d98 Mon Sep 17 00:00:00 2001 -From: George Dunlap <george.dunlap@citrix.com> -Date: Thu, 15 Jun 2017 12:05:14 +0100 -Subject: [PATCH 2/3] gnttab: Avoid potential double-put of maptrack entry - -Each grant mapping for a particular domain is tracked by an in-Xen -"maptrack" entry. This entry is is referenced by a "handle", which is -given to the guest when it calls gnttab_map_grant_ref(). - -There are two types of mapping a particular handle can refer to: -GNTMAP_host_map and GNTMAP_device_map. A given -gnttab_unmap_grant_ref() call can remove either only one or both of -these entries. When a particular handle has no entries left, it must -be freed. - -gnttab_unmap_grant_ref() loops through its grant unmap request list -twice. It first removes entries from any host pagetables and (if -appropraite) iommus; then it does a single domain TLB flush; then it -does the clean-up, including telling the granter that entries are no -longer being used (if appropriate). - -At the moment, it's during the first pass that the maptrack flags are -cleared, but the second pass that the maptrack entry is freed. - -Unfortunately this allows the following race, which results in a -double-free: - - A: (pass 1) clear host_map - B: (pass 1) clear device_map - A: (pass 2) See that maptrack entry has no mappings, free it - B: (pass 2) See that maptrack entry has no mappings, free it # - -Unfortunately, unlike the active entry pinning update, we can't simply -move the maptrack flag changes to the second half, because the -maptrack flags are used to determine if iommu entries need to be -added: a domain's iommu must never have fewer permissions than the -maptrack flags indicate, or a subsequent map_grant_ref() might fail to -add the necessary iommu entries. - -Instead, free the maptrack entry in the first pass if there are no -further mappings. - -This is part of XSA-218. - -Reported-by: Jan Beulich <jbeulich.com> -Signed-off-by: George Dunlap <george.dunlap@citrix.com> -Signed-off-by: Jan Beulich <jbeulich@suse.com> ---- - xen/common/grant_table.c | 77 +++++++++++++++++++++++++++++++++--------------- - 1 file changed, 53 insertions(+), 24 deletions(-) - -diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c -index d80bd49..ba10e76 100644 ---- a/xen/common/grant_table.c -+++ b/xen/common/grant_table.c -@@ -98,8 +98,8 @@ struct gnttab_unmap_common { - /* Shared state beteen *_unmap and *_unmap_complete */ - u16 flags; - unsigned long frame; -- struct grant_mapping *map; - struct domain *rd; -+ grant_ref_t ref; - }; - - /* Number of unmap operations that are done between each tlb flush */ -@@ -1079,6 +1079,8 @@ __gnttab_unmap_common( - struct grant_table *lgt, *rgt; - struct active_grant_entry *act; - s16 rc = 0; -+ struct grant_mapping *map; -+ bool put_handle = false; - - ld = current->domain; - lgt = ld->grant_table; -@@ -1092,11 +1094,11 @@ __gnttab_unmap_common( - return; - } - -- op->map = &maptrack_entry(lgt, op->handle); -+ map = &maptrack_entry(lgt, op->handle); - - grant_read_lock(lgt); - -- if ( unlikely(!read_atomic(&op->map->flags)) ) -+ if ( unlikely(!read_atomic(&map->flags)) ) - { - grant_read_unlock(lgt); - gdprintk(XENLOG_INFO, "Zero flags for handle (%d).\n", op->handle); -@@ -1104,7 +1106,7 @@ __gnttab_unmap_common( - return; - } - -- dom = op->map->domid; -+ dom = map->domid; - grant_read_unlock(lgt); - - if ( unlikely((rd = rcu_lock_domain_by_id(dom)) == NULL) ) -@@ -1129,16 +1131,43 @@ __gnttab_unmap_common( - - grant_read_lock(rgt); - -- op->flags = read_atomic(&op->map->flags); -- if ( unlikely(!op->flags) || unlikely(op->map->domid != dom) ) -+ op->rd = rd; -+ op->ref = map->ref; -+ -+ /* -+ * We can't assume there was no racing unmap for this maptrack entry, -+ * and hence we can't assume map->ref is valid for rd. While the checks -+ * below (with the active entry lock held) will reject any such racing -+ * requests, we still need to make sure we don't attempt to acquire an -+ * invalid lock. -+ */ -+ smp_rmb(); -+ if ( unlikely(op->ref >= nr_grant_entries(rgt)) ) - { - gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle); - rc = GNTST_bad_handle; -- goto unmap_out; -+ goto unlock_out; - } - -- op->rd = rd; -- act = active_entry_acquire(rgt, op->map->ref); -+ act = active_entry_acquire(rgt, op->ref); -+ -+ /* -+ * Note that we (ab)use the active entry lock here to protect against -+ * multiple unmaps of the same mapping here. We don't want to hold lgt's -+ * lock, and we only hold rgt's lock for reading (but the latter wouldn't -+ * be the right one anyway). Hence the easiest is to rely on a lock we -+ * hold anyway; see docs/misc/grant-tables.txt's "Locking" section. -+ */ -+ -+ op->flags = read_atomic(&map->flags); -+ smp_rmb(); -+ if ( unlikely(!op->flags) || unlikely(map->domid != dom) || -+ unlikely(map->ref != op->ref) ) -+ { -+ gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle); -+ rc = GNTST_bad_handle; -+ goto act_release_out; -+ } - - if ( op->frame == 0 ) - { -@@ -1151,7 +1180,7 @@ __gnttab_unmap_common( - "Bad frame number doesn't match gntref. (%lx != %lx)\n", - op->frame, act->frame); - -- op->map->flags &= ~GNTMAP_device_map; -+ map->flags &= ~GNTMAP_device_map; - } - - if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) ) -@@ -1161,14 +1190,23 @@ __gnttab_unmap_common( - op->flags)) < 0 ) - goto act_release_out; - -- op->map->flags &= ~GNTMAP_host_map; -+ map->flags &= ~GNTMAP_host_map; -+ } -+ -+ if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) ) -+ { -+ map->flags = 0; -+ put_handle = true; - } - - act_release_out: - active_entry_release(act); -- unmap_out: -+ unlock_out: - grant_read_unlock(rgt); - -+ if ( put_handle ) -+ put_maptrack_handle(lgt, op->handle); -+ - if ( rc == GNTST_okay && gnttab_need_iommu_mapping(ld) ) - { - unsigned int kind; -@@ -1205,7 +1243,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op) - grant_entry_header_t *sha; - struct page_info *pg; - uint16_t *status; -- bool_t put_handle = 0; - - if ( rd == NULL ) - { -@@ -1226,13 +1263,13 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op) - if ( rgt->gt_version == 0 ) - goto unlock_out; - -- act = active_entry_acquire(rgt, op->map->ref); -- sha = shared_entry_header(rgt, op->map->ref); -+ act = active_entry_acquire(rgt, op->ref); -+ sha = shared_entry_header(rgt, op->ref); - - if ( rgt->gt_version == 1 ) - status = &sha->flags; - else -- status = &status_entry(rgt, op->map->ref); -+ status = &status_entry(rgt, op->ref); - - if ( unlikely(op->frame != act->frame) ) - { -@@ -1289,9 +1326,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op) - act->pin -= GNTPIN_hstw_inc; - } - -- if ( (op->map->flags & (GNTMAP_device_map|GNTMAP_host_map)) == 0 ) -- put_handle = 1; -- - if ( ((act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0) && - !(op->flags & GNTMAP_readonly) ) - gnttab_clear_flag(_GTF_writing, status); -@@ -1304,11 +1338,6 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op) - unlock_out: - grant_read_unlock(rgt); - -- if ( put_handle ) -- { -- op->map->flags = 0; -- put_maptrack_handle(ld->grant_table, op->handle); -- } - rcu_unlock_domain(rd); - } - --- -2.1.4 - |