Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP1:GA
xen.10697
59491937-gnttab-__gnttab_unmap_common_complete-...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 59491937-gnttab-__gnttab_unmap_common_complete-is-all-or-nothing.patch of Package xen.10697
# Commit 11fc7ccb7217ccfb79edb727d1349618bcb0602d # Date 2017-06-20 14:46:47 +0200 # Author Jan Beulich <jbeulich@suse.com> # Committer Jan Beulich <jbeulich@suse.com> 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> --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -75,7 +75,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; @@ -714,7 +714,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) ) @@ -848,6 +849,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; @@ -897,8 +899,22 @@ __gnttab_unmap_common( rgt = rd->grant_table; double_gt_lock(lgt, rgt); - op->flags = map->flags; - if ( unlikely(!op->flags) || unlikely(map->domid != dom) ) + 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 unmap_out; + } + + flags = map->flags; + if ( unlikely(!flags) || unlikely(map->domid != dom) ) { gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle); rc = GNTST_bad_handle; @@ -911,24 +927,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(unmap_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(unmap_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 unmap_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)) ) @@ -964,7 +983,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; @@ -981,13 +1000,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; } @@ -997,9 +1012,6 @@ __gnttab_unmap_common_complete(struct gn rgt = rd->grant_table; spin_lock(&rgt->lock); - if ( rgt->gt_version == 0 ) - goto unmap_out; - act = &active_entry(rgt, op->ref); sha = shared_entry_header(rgt, op->ref); @@ -1008,70 +1020,49 @@ __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 unmap_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 ) - { - /* - * 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 unmap_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); - unmap_out: spin_unlock(&rgt->lock); rcu_unlock_domain(rd); } @@ -1086,6 +1077,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; @@ -1151,6 +1143,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; @@ -2795,7 +2788,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); } --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -10,7 +10,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); --- a/xen/include/asm-x86/grant_table.h +++ b/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
