Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:GA
xen.5001
xsa224-0004-gnttab-__gnttab_unmap_common_comple...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File xsa224-0004-gnttab-__gnttab_unmap_common_complete-is-all-or-noth.patch of Package xen.5001
From 3ad26b95cd9bacedad5ba503515cf6e618162be1 Mon Sep 17 00:00:00 2001 From: Jan Beulich <jbeulich@suse.com> Date: Thu, 15 Jun 2017 16:25:27 +0100 Subject: [PATCH 4/4] gnttab: __gnttab_unmap_common_complete() is all-or-nothing All failures have to be detected in __gnttab_unmap_common(), the completion function must not skip part of its processing. In particular the GNTMAP_device_map related putting of page references and adjustment of pin count must not occur if __gnttab_unmap_common() signaled an error. Furthermore the function must not make adjustments to global state (here: clearing GNTTAB_device_map) before all possibly failing operations have been performed. There's one exception for IOMMU related failures: As IOMMU manipulation occurs after GNTMAP_*_map have been cleared already, the related page reference and pin count adjustments need to be done nevertheless. A fundamental requirement for the correctness of this is that iommu_{,un}map_page() crash any affected DomU in case of failure. The version check appears to be pointless (or could perhaps be a BUG_ON() or ASSERT()), but for the moment also move it. This is part of XSA-224. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- xen/common/grant_table.c | 108 ++++++++++++++++++-------------------- xen/include/asm-arm/grant_table.h | 2 +- xen/include/asm-x86/grant_table.h | 5 +- 3 files changed, 55 insertions(+), 60 deletions(-) Index: xen-4.7.2-testing/xen/common/grant_table.c =================================================================== --- xen-4.7.2-testing.orig/xen/common/grant_table.c +++ xen-4.7.2-testing/xen/common/grant_table.c @@ -96,7 +96,7 @@ struct gnttab_unmap_common { int16_t status; /* Shared state beteen *_unmap and *_unmap_complete */ - u16 flags; + u16 done; unsigned long frame; struct domain *rd; grant_ref_t ref; @@ -948,7 +948,8 @@ __gnttab_map_grant_ref( refcnt++; } - if ( gnttab_host_mapping_get_page_type(op, ld, rd) ) + if ( gnttab_host_mapping_get_page_type(op->flags & GNTMAP_readonly, + ld, rd) ) { if ( (owner == dom_cow) || !get_page_type(pg, PGT_writable_page) ) @@ -1095,6 +1096,7 @@ __gnttab_unmap_common( struct active_grant_entry *act; s16 rc = 0; struct grant_mapping *map; + unsigned int flags; bool_t put_handle = 0; ld = current->domain; @@ -1145,6 +1147,20 @@ __gnttab_unmap_common( grant_read_lock(rgt); + if ( rgt->gt_version == 0 ) + { + /* + * This ought to be impossible, as such a mapping should not have + * been established (see the nr_grant_entries(rgt) bounds check in + * __gnttab_map_grant_ref()). Doing this check only in + * __gnttab_unmap_common_complete() - as it used to be done - would, + * however, be too late. + */ + rc = GNTST_bad_gntref; + flags = 0; + goto unlock_out; + } + op->rd = rd; op->ref = map->ref; @@ -1160,6 +1176,7 @@ __gnttab_unmap_common( { gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle); rc = GNTST_bad_handle; + flags = 0; goto unlock_out; } @@ -1173,9 +1190,9 @@ __gnttab_unmap_common( * hold anyway; see docs/misc/grant-tables.txt's "Locking" section. */ - op->flags = read_atomic(&map->flags); + flags = read_atomic(&map->flags); smp_rmb(); - if ( unlikely(!op->flags) || unlikely(map->domid != dom) || + if ( unlikely(!flags) || unlikely(map->domid != dom) || unlikely(map->ref != op->ref) ) { gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle); @@ -1185,24 +1202,27 @@ __gnttab_unmap_common( op->frame = act->frame; - if ( op->dev_bus_addr ) - { - if ( unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) ) - PIN_FAIL(act_release_out, GNTST_general_error, - "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n", - op->dev_bus_addr, pfn_to_paddr(act->frame)); - - map->flags &= ~GNTMAP_device_map; - } + if ( op->dev_bus_addr && + unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) ) + PIN_FAIL(act_release_out, GNTST_general_error, + "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n", + op->dev_bus_addr, pfn_to_paddr(act->frame)); - if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) ) + if ( op->host_addr && (flags & GNTMAP_host_map) ) { if ( (rc = replace_grant_host_mapping(op->host_addr, op->frame, op->new_addr, - op->flags)) < 0 ) + flags)) < 0 ) goto act_release_out; map->flags &= ~GNTMAP_host_map; + op->done |= GNTMAP_host_map | (flags & GNTMAP_readonly); + } + + if ( op->dev_bus_addr && (flags & GNTMAP_device_map) ) + { + map->flags &= ~GNTMAP_device_map; + op->done |= GNTMAP_device_map | (flags & GNTMAP_readonly); } if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) ) @@ -1239,7 +1259,7 @@ __gnttab_unmap_common( } /* If just unmapped a writable mapping, mark as dirtied */ - if ( rc == GNTST_okay && !(op->flags & GNTMAP_readonly) ) + if ( rc == GNTST_okay && !(flags & GNTMAP_readonly) ) gnttab_mark_dirty(rd, op->frame); op->status = rc; @@ -1256,13 +1276,9 @@ __gnttab_unmap_common_complete(struct gn struct page_info *pg; uint16_t *status; - if ( rd == NULL ) + if ( !op->done ) { - /* - * Suggests that __gntab_unmap_common failed in - * rcu_lock_domain_by_id() or earlier, and so we have nothing - * to complete - */ + /* __gntab_unmap_common() didn't do anything - nothing to complete. */ return; } @@ -1272,8 +1288,6 @@ __gnttab_unmap_common_complete(struct gn rgt = rd->grant_table; grant_read_lock(rgt); - if ( rgt->gt_version == 0 ) - goto unlock_out; act = active_entry_acquire(rgt, op->ref); sha = shared_entry_header(rgt, op->ref); @@ -1283,72 +1297,50 @@ __gnttab_unmap_common_complete(struct gn else status = &status_entry(rgt, op->ref); - if ( op->dev_bus_addr && - unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) ) - { - /* - * Suggests that __gntab_unmap_common failed early and so - * nothing further to do - */ - goto act_release_out; - } - pg = mfn_to_page(op->frame); - if ( op->dev_bus_addr && (op->flags & GNTMAP_device_map) ) + if ( op->done & GNTMAP_device_map ) { if ( !is_iomem_page(act->frame) ) { - if ( op->flags & GNTMAP_readonly ) + if ( op->done & GNTMAP_readonly ) put_page(pg); else put_page_and_type(pg); } ASSERT(act->pin & (GNTPIN_devw_mask | GNTPIN_devr_mask)); - if ( op->flags & GNTMAP_readonly ) + if ( op->done & GNTMAP_readonly ) act->pin -= GNTPIN_devr_inc; else act->pin -= GNTPIN_devw_inc; } - if ( (op->host_addr != 0) && (op->flags & GNTMAP_host_map) ) + if ( op->done & GNTMAP_host_map ) { - if ( op->status != 0 ) + if ( !is_iomem_page(op->frame) ) { - /* - * Suggests that __gntab_unmap_common failed in - * replace_grant_host_mapping() or IOMMU handling, so nothing - * further to do (short of re-establishing the mapping in the - * latter case). - */ - goto act_release_out; - } - - if ( !is_iomem_page(op->frame) ) - { - if ( gnttab_host_mapping_get_page_type(op, ld, rd) ) + if ( gnttab_host_mapping_get_page_type(op->done & GNTMAP_readonly, + ld, rd) ) put_page_type(pg); put_page(pg); } ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask)); - if ( op->flags & GNTMAP_readonly ) + if ( op->done & GNTMAP_readonly ) act->pin -= GNTPIN_hstr_inc; else act->pin -= GNTPIN_hstw_inc; } if ( ((act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0) && - !(op->flags & GNTMAP_readonly) ) + !(op->done & GNTMAP_readonly) ) gnttab_clear_flag(_GTF_writing, status); if ( act->pin == 0 ) gnttab_clear_flag(_GTF_reading, status); - act_release_out: active_entry_release(act); - unlock_out: grant_read_unlock(rgt); rcu_unlock_domain(rd); @@ -1364,6 +1356,7 @@ __gnttab_unmap_grant_ref( common->handle = op->handle; /* Intialise these in case common contains old state */ + common->done = 0; common->new_addr = 0; common->rd = NULL; common->frame = 0; @@ -1429,6 +1422,7 @@ __gnttab_unmap_and_replace( common->handle = op->handle; /* Intialise these in case common contains old state */ + common->done = 0; common->dev_bus_addr = 0; common->rd = NULL; common->frame = 0; @@ -3387,7 +3381,9 @@ gnttab_release_mappings( if ( gnttab_release_host_mappings(d) && !is_iomem_page(act->frame) ) { - if ( gnttab_host_mapping_get_page_type(map, d, rd) ) + if ( gnttab_host_mapping_get_page_type((map->flags & + GNTMAP_readonly), + d, rd) ) put_page_type(pg); put_page(pg); } Index: xen-4.7.2-testing/xen/include/asm-arm/grant_table.h =================================================================== --- xen-4.7.2-testing.orig/xen/include/asm-arm/grant_table.h +++ xen-4.7.2-testing/xen/include/asm-arm/grant_table.h @@ -9,7 +9,7 @@ void gnttab_clear_flag(unsigned long nr, int create_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, unsigned int flags, unsigned int cache_flags); -#define gnttab_host_mapping_get_page_type(op, d, rd) (0) +#define gnttab_host_mapping_get_page_type(ro, ld, rd) (0) int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, unsigned long new_gpaddr, unsigned int flags); void gnttab_mark_dirty(struct domain *d, unsigned long l); Index: xen-4.7.2-testing/xen/include/asm-x86/grant_table.h =================================================================== --- xen-4.7.2-testing.orig/xen/include/asm-x86/grant_table.h +++ xen-4.7.2-testing/xen/include/asm-x86/grant_table.h @@ -58,9 +58,8 @@ static inline void gnttab_clear_flag(uns } /* Foreign mappings of HHVM-guest pages do not modify the type count. */ -#define gnttab_host_mapping_get_page_type(op, ld, rd) \ - (!((op)->flags & GNTMAP_readonly) && \ - (((ld) == (rd)) || !paging_mode_external(rd))) +#define gnttab_host_mapping_get_page_type(ro, ld, rd) \ + (!(ro) && (((ld) == (rd)) || !paging_mode_external(rd))) /* Done implicitly when page tables are destroyed. */ #define gnttab_release_host_mappings(domain) ( paging_mode_external(domain) )
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
