system/xen: XSA 226-228 and 230 update.
Signed-off-by: Mario Preksavec <mario@slackware.hr>
This commit is contained in:
parent
cded8a857b
commit
d226051fa7
|
@ -24,7 +24,7 @@
|
|||
|
||||
PRGNAM=xen
|
||||
VERSION=${VERSION:-4.9.0}
|
||||
BUILD=${BUILD:-1}
|
||||
BUILD=${BUILD:-2}
|
||||
TAG=${TAG:-_SBo}
|
||||
|
||||
SEABIOS=${SEABIOS:-1.10.0}
|
||||
|
|
|
@ -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:
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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 )
|
Loading…
Reference in New Issue