diff options
Diffstat (limited to 'system/xen/xsa')
| -rw-r--r-- | system/xen/xsa/xsa226.patch | 133 | ||||
| -rw-r--r-- | system/xen/xsa/xsa227.patch | 52 | ||||
| -rw-r--r-- | system/xen/xsa/xsa228.patch | 198 | ||||
| -rw-r--r-- | system/xen/xsa/xsa230.patch | 38 |
4 files changed, 421 insertions, 0 deletions
diff --git a/system/xen/xsa/xsa226.patch b/system/xen/xsa/xsa226.patch new file mode 100644 index 0000000000..48fae12172 --- /dev/null +++ b/system/xen/xsa/xsa226.patch @@ -0,0 +1,133 @@ +From: Andrew Cooper <andrew.cooper3@citrix.com> +Subject: grant_table: Default to v1, and disallow transitive grants + +The reference counting and locking discipline for transitive grants is broken. +Their use is therefore declared out of security support. + +This is XSA-226. + +Transitive grants are expected to be unconditionally available with grant +table v2. Hiding transitive grants alone is an ABI breakage for the guest. +Modern versions of Linux and the Windows PV drivers use grant table v1, but +older versions did use v2. + +In principle, disabling gnttab v2 entirely is the safer way to cause guests to +avoid using transitive grants. However, some older guests which defaulted to +using gnttab v2 don't tolerate falling back from v2 to v1 over migrate. + +This patch introduces a new command line option to control grant table +behaviour. One suboption allows a choice of the maximum grant table version +Xen will allow the guest to use, and defaults to v2. A different suboption +independently controls whether transitive grants can be used. + +The default case is: + + gnttab=max_ver:2 + +To disable gnttab v2 entirely, use: + + gnttab=max_ver:1 + +To allow gnttab v2 and transitive grants, use: + + gnttab=max_ver:2,transitive + +Reported-by: Jan Beulich <jbeulich@suse.com> +Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> +diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown +index 4002eab..af079b4 100644 +--- a/docs/misc/xen-command-line.markdown ++++ b/docs/misc/xen-command-line.markdown +@@ -868,6 +868,22 @@ Controls EPT related features. + + Specify which console gdbstub should use. See **console**. + ++### gnttab ++> `= List of [ max_ver:<integer>, transitive ]` ++ ++> Default: `gnttab=max_ver:2,no-transitive` ++ ++Control various aspects of the grant table behaviour available to guests. ++ ++* `max_ver` Select the maximum grant table version to offer to guests. Valid ++version are 1 and 2. ++* `transitive` Permit or disallow the use of transitive grants. Note that the ++use of grant table v2 without transitive grants is an ABI breakage from the ++guests point of view. ++ ++*Warning:* ++Due to XSA-226, the use of transitive grants is outside of security support. ++ + ### gnttab\_max\_frames + > `= <integer>` + +diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c +index ae34547..87131f8 100644 +--- a/xen/common/grant_table.c ++++ b/xen/common/grant_table.c +@@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", max_nr_grant_frames); + unsigned int __read_mostly max_grant_frames; + integer_param("gnttab_max_frames", max_grant_frames); + ++static unsigned int __read_mostly opt_gnttab_max_version = 2; ++static bool __read_mostly opt_transitive_grants; ++ ++static void __init parse_gnttab(char *s) ++{ ++ char *ss; ++ ++ do { ++ ss = strchr(s, ','); ++ if ( ss ) ++ *ss = '\0'; ++ ++ if ( !strncmp(s, "max_ver:", 8) ) ++ { ++ long ver = simple_strtol(s + 8, NULL, 10); ++ ++ if ( ver >= 1 && ver <= 2 ) ++ opt_gnttab_max_version = ver; ++ } ++ else ++ { ++ bool val = !!strncmp(s, "no-", 3); ++ ++ if ( !val ) ++ s += 3; ++ ++ if ( !strcmp(s, "transitive") ) ++ opt_transitive_grants = val; ++ } ++ ++ s = ss + 1; ++ } while ( ss ); ++} ++ ++custom_param("gnttab", parse_gnttab); ++ + /* The maximum number of grant mappings is defined as a multiplier of the + * maximum number of grant table entries. This defines the multiplier used. + * Pretty arbitrary. [POLICY] +@@ -2191,6 +2227,10 @@ __acquire_grant_for_copy( + } + else if ( (shah->flags & GTF_type_mask) == GTF_transitive ) + { ++ if ( !opt_transitive_grants ) ++ PIN_FAIL(unlock_out_clear, GNTST_general_error, ++ "transitive grant disallowed by policy\n"); ++ + if ( !allow_transitive ) + PIN_FAIL(unlock_out_clear, GNTST_general_error, + "transitive grant when transitivity not allowed\n"); +@@ -3159,7 +3199,10 @@ do_grant_table_op( + } + case GNTTABOP_set_version: + { +- rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t)); ++ if ( opt_gnttab_max_version == 1 ) ++ rc = -ENOSYS; /* Behave as before set_version was introduced. */ ++ else ++ rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t)); + break; + } + case GNTTABOP_get_status_frames: diff --git a/system/xen/xsa/xsa227.patch b/system/xen/xsa/xsa227.patch new file mode 100644 index 0000000000..86aa41e2d4 --- /dev/null +++ b/system/xen/xsa/xsa227.patch @@ -0,0 +1,52 @@ +From fa7268b94f8a0a7792ee12d5b8e23a60e52a3a84 Mon Sep 17 00:00:00 2001 +From: Andrew Cooper <andrew.cooper3@citrix.com> +Date: Tue, 20 Jun 2017 19:18:54 +0100 +Subject: [PATCH] x86/grant: Disallow misaligned PTEs + +Pagetable entries must be aligned to function correctly. Disallow attempts +from the guest to have a grant PTE created at a misaligned address, which +would result in corruption of the L1 table with largely-guest-controlled +values. + +This is XSA-227 + +Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> +Reviewed-by: Jan Beulich <jbeulich@suse.com> +--- + xen/arch/x86/mm.c | 13 +++++++++++++ + 1 file changed, 13 insertions(+) + +diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c +index 97b3b4b..00f517a 100644 +--- a/xen/arch/x86/mm.c ++++ b/xen/arch/x86/mm.c +@@ -3763,6 +3763,9 @@ static int create_grant_pte_mapping( + l1_pgentry_t ol1e; + struct domain *d = v->domain; + ++ if ( !IS_ALIGNED(pte_addr, sizeof(nl1e)) ) ++ return GNTST_general_error; ++ + adjust_guest_l1e(nl1e, d); + + gmfn = pte_addr >> PAGE_SHIFT; +@@ -3819,6 +3822,16 @@ static int destroy_grant_pte_mapping( + struct page_info *page; + l1_pgentry_t ol1e; + ++ /* ++ * addr comes from Xen's active_entry tracking so isn't guest controlled, ++ * but it had still better be PTE-aligned. ++ */ ++ if ( !IS_ALIGNED(addr, sizeof(ol1e)) ) ++ { ++ ASSERT_UNREACHABLE(); ++ return GNTST_general_error; ++ } ++ + gmfn = addr >> PAGE_SHIFT; + page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); + +-- +2.1.4 + diff --git a/system/xen/xsa/xsa228.patch b/system/xen/xsa/xsa228.patch new file mode 100644 index 0000000000..65add3a588 --- /dev/null +++ b/system/xen/xsa/xsa228.patch @@ -0,0 +1,198 @@ +From 9a52c78eb4ff7836bf7ac9ecd918b289cead1f3f Mon Sep 17 00:00:00 2001 +From: Jan Beulich <jbeulich@suse.com> +Date: Mon, 31 Jul 2017 15:17:56 +0100 +Subject: [PATCH] gnttab: split maptrack lock to make it fulfill its purpose + again + +The way the lock is currently being used in get_maptrack_handle(), it +protects only the maptrack limit: The function acts on current's list +only, so races on list accesses are impossible even without the lock. + +Otoh list access races are possible between __get_maptrack_handle() and +put_maptrack_handle(), due to the invocation of the former for other +than current from steal_maptrack_handle(). Introduce a per-vCPU lock +for list accesses to become race free again. This lock will be +uncontended except when it becomes necessary to take the steal path, +i.e. in the common case there should be no meaningful performance +impact. + +When in get_maptrack_handle adds a stolen entry to a fresh, empty, +freelist, we think that there is probably no concurrency. However, +this is not a fast path and adding the locking there makes the code +clearly correct. + +Also, while we are here: the stolen maptrack_entry's tail pointer was +not properly set. Set it. + +This is XSA-228. + +Reported-by: Ian Jackson <ian.jackson@eu.citrix.com> +Signed-off-by: Jan Beulich <jbeulich@suse.com> +Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> +--- + docs/misc/grant-tables.txt | 7 ++++++- + xen/common/grant_table.c | 30 ++++++++++++++++++++++++------ + xen/include/xen/grant_table.h | 2 +- + xen/include/xen/sched.h | 1 + + 4 files changed, 32 insertions(+), 8 deletions(-) + +diff --git a/docs/misc/grant-tables.txt b/docs/misc/grant-tables.txt +index 417ce2d..64da5cf 100644 +--- a/docs/misc/grant-tables.txt ++++ b/docs/misc/grant-tables.txt +@@ -87,7 +87,8 @@ is complete. + inconsistent grant table state such as current + version, partially initialized active table pages, + etc. +- grant_table->maptrack_lock : spinlock used to protect the maptrack free list ++ grant_table->maptrack_lock : spinlock used to protect the maptrack limit ++ v->maptrack_freelist_lock : spinlock used to protect the maptrack free list + active_grant_entry->lock : spinlock used to serialize modifications to + active entries + +@@ -102,6 +103,10 @@ is complete. + The maptrack free list is protected by its own spinlock. The maptrack + lock may be locked while holding the grant table lock. + ++ The maptrack_freelist_lock is an innermost lock. It may be locked ++ while holding other locks, but no other locks may be acquired within ++ it. ++ + Active entries are obtained by calling active_entry_acquire(gt, ref). + This function returns a pointer to the active entry after locking its + spinlock. The caller must hold the grant table read lock before +diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c +index ae34547..ee33bd8 100644 +--- a/xen/common/grant_table.c ++++ b/xen/common/grant_table.c +@@ -304,11 +304,16 @@ __get_maptrack_handle( + { + unsigned int head, next, prev_head; + ++ spin_lock(&v->maptrack_freelist_lock); ++ + do { + /* No maptrack pages allocated for this VCPU yet? */ + head = read_atomic(&v->maptrack_head); + if ( unlikely(head == MAPTRACK_TAIL) ) ++ { ++ spin_unlock(&v->maptrack_freelist_lock); + return -1; ++ } + + /* + * Always keep one entry in the free list to make it easier to +@@ -316,12 +321,17 @@ __get_maptrack_handle( + */ + next = read_atomic(&maptrack_entry(t, head).ref); + if ( unlikely(next == MAPTRACK_TAIL) ) ++ { ++ spin_unlock(&v->maptrack_freelist_lock); + return -1; ++ } + + prev_head = head; + head = cmpxchg(&v->maptrack_head, prev_head, next); + } while ( head != prev_head ); + ++ spin_unlock(&v->maptrack_freelist_lock); ++ + return head; + } + +@@ -380,6 +390,8 @@ put_maptrack_handle( + /* 2. Add entry to the tail of the list on the original VCPU. */ + v = currd->vcpu[maptrack_entry(t, handle).vcpu]; + ++ spin_lock(&v->maptrack_freelist_lock); ++ + cur_tail = read_atomic(&v->maptrack_tail); + do { + prev_tail = cur_tail; +@@ -388,6 +400,8 @@ put_maptrack_handle( + + /* 3. Update the old tail entry to point to the new entry. */ + write_atomic(&maptrack_entry(t, prev_tail).ref, handle); ++ ++ spin_unlock(&v->maptrack_freelist_lock); + } + + static inline int +@@ -411,10 +425,6 @@ get_maptrack_handle( + */ + if ( nr_maptrack_frames(lgt) >= max_maptrack_frames ) + { +- /* +- * Can drop the lock since no other VCPU can be adding a new +- * frame once they've run out. +- */ + spin_unlock(&lgt->maptrack_lock); + + /* +@@ -426,8 +436,12 @@ get_maptrack_handle( + handle = steal_maptrack_handle(lgt, curr); + if ( handle == -1 ) + return -1; ++ spin_lock(&curr->maptrack_freelist_lock); ++ maptrack_entry(lgt, handle).ref = MAPTRACK_TAIL; + curr->maptrack_tail = handle; +- write_atomic(&curr->maptrack_head, handle); ++ if ( curr->maptrack_head == MAPTRACK_TAIL ) ++ write_atomic(&curr->maptrack_head, handle); ++ spin_unlock(&curr->maptrack_freelist_lock); + } + return steal_maptrack_handle(lgt, curr); + } +@@ -460,12 +474,15 @@ get_maptrack_handle( + smp_wmb(); + lgt->maptrack_limit += MAPTRACK_PER_PAGE; + ++ spin_unlock(&lgt->maptrack_lock); ++ spin_lock(&curr->maptrack_freelist_lock); ++ + 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); ++ spin_unlock(&curr->maptrack_freelist_lock); + + return handle; + } +@@ -3475,6 +3492,7 @@ grant_table_destroy( + + void grant_table_init_vcpu(struct vcpu *v) + { ++ spin_lock_init(&v->maptrack_freelist_lock); + v->maptrack_head = MAPTRACK_TAIL; + v->maptrack_tail = MAPTRACK_TAIL; + } +diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h +index 4e77899..100f2b3 100644 +--- a/xen/include/xen/grant_table.h ++++ b/xen/include/xen/grant_table.h +@@ -78,7 +78,7 @@ struct grant_table { + /* Mapping tracking table per vcpu. */ + struct grant_mapping **maptrack; + unsigned int maptrack_limit; +- /* Lock protecting the maptrack page list, head, and limit */ ++ /* Lock protecting the maptrack limit */ + spinlock_t maptrack_lock; + /* The defined versions are 1 and 2. Set to 0 if we don't know + what version to use yet. */ +diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h +index 6673b27..8690f29 100644 +--- a/xen/include/xen/sched.h ++++ b/xen/include/xen/sched.h +@@ -230,6 +230,7 @@ struct vcpu + int controller_pause_count; + + /* Grant table map tracking. */ ++ spinlock_t maptrack_freelist_lock; + unsigned int maptrack_head; + unsigned int maptrack_tail; + +-- +2.1.4 + diff --git a/system/xen/xsa/xsa230.patch b/system/xen/xsa/xsa230.patch new file mode 100644 index 0000000000..c3b50c8aaa --- /dev/null +++ b/system/xen/xsa/xsa230.patch @@ -0,0 +1,38 @@ +From: Jan Beulich <jbeulich@suse.com> +Subject: gnttab: correct pin status fixup for copy + +Regardless of copy operations only setting GNTPIN_hst*, GNTPIN_dev* +also need to be taken into account when deciding whether to clear +_GTF_{read,writ}ing. At least for consistency with code elsewhere the +read part better doesn't use any mask at all. + +This is XSA-230. + +Signed-off-by: Jan Beulich <jbeulich@suse.com> +Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> +diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c +index ae34547..9c9d33c 100644 +--- a/xen/common/grant_table.c ++++ b/xen/common/grant_table.c +@@ -2107,10 +2107,10 @@ __release_grant_for_copy( + static void __fixup_status_for_copy_pin(const struct active_grant_entry *act, + uint16_t *status) + { +- if ( !(act->pin & GNTPIN_hstw_mask) ) ++ if ( !(act->pin & (GNTPIN_hstw_mask | GNTPIN_devw_mask)) ) + gnttab_clear_flag(_GTF_writing, status); + +- if ( !(act->pin & GNTPIN_hstr_mask) ) ++ if ( !act->pin ) + gnttab_clear_flag(_GTF_reading, status); + } + +@@ -2318,7 +2318,7 @@ __acquire_grant_for_copy( + + unlock_out_clear: + if ( !(readonly) && +- !(act->pin & GNTPIN_hstw_mask) ) ++ !(act->pin & (GNTPIN_hstw_mask | GNTPIN_devw_mask)) ) + gnttab_clear_flag(_GTF_writing, status); + + if ( !act->pin ) |