* Xen Security Advisory 269 v3 (CVE-2018-15468) - x86: Incorrect MSR_DEBUGCTL handling lets guests enable BTS
@ 2018-08-20 9:47 Xen.org security team
0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2018-08-20 9:47 UTC (permalink / raw)
To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team
[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2018-15468 / XSA-269
version 3
x86: Incorrect MSR_DEBUGCTL handling lets guests enable BTS
UPDATES IN VERSION 3
====================
CVE assigned.
ISSUE DESCRIPTION
=================
The DEBUGCTL MSR contains several debugging features, some of which virtualise
cleanly, but some do not. In particular, Branch Trace Store is not
virtualised by the processor, and software has to be careful to configure it
suitably not to lock up the core. As a result, it must only be available to
fully trusted guests.
Unfortunately, in the case that vPMU is disabled, all value checking was
skipped, allowing the guest to chose any MSR_DEBUGCTL setting it likes.
IMPACT
======
A malicious or buggy guest administrator can lock up the entire host, causing
a Denial of Service.
VULNERABLE SYSTEMS
==================
Xen versions 4.6 and later are vulnerable.
Only systems using Intel CPUs are affected. ARM and AMD systems are
unaffected.
Only x86 HVM or PVH guests can exploit the vulnerability. x86 PV guests
cannot exploit the vulnerability.
MITIGATION
==========
Running only x86 PV guests avoids the vulnerability.
CREDITS
=======
This issue was discovered by Andrew Cooper of Citrix.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa269.patch xen-unstable
xsa269-4.11.patch Xen 4.11
xsa269-4.10.patch 4.10, 4.9
xsa269-4.8.patch Xen 4.8, 4.7, 4.6
$ sha256sum xsa269*
4733d09bb63523744ca2ee172e2fade0c39082c15d9a746144f279cf1359b723 xsa269.meta
5a5fe36f1f876a5029493e7fa191436fd021929aaba2d820636df17f4ed20113 xsa269.patch
ea11cef818050bca13d4eb89294627c97e4cdb830124f679e77d37a44a370286 xsa269-4.8.patch
45ba1823530f329dd73088b77098e686b32f5daac0bc5177b2afea09f8c3593a xsa269-4.10.patch
e0ca060311fb9ba3247e2fe65bca4806a131644f8894fd08be374904904b1944 xsa269-4.11.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAEBCAAGBQJbeo4KAAoJEIP+FMlX6CvZfakIAJRgw9LWW7fnr0WX11dt/Rm1
GgBxMWS7DrnBPBjE7GqhtqgFyvIVHBnWEEj1WW1WvHWIV/XIbV8GKOi6ecfF5p3o
vK/a/8S0qOSOtOPZZJkZGuZn6pNd9V0Ynx296Hn6DKildBBEkGSXoWo67ViaxrP2
iPzhYukDRYlqjF5pYfPr7Zek+RodtB+rxJEKMpDDIW8aeA3hnsOZNXAmr5n+Q465
rNojqJDV5Zwuli+L0SVzmtkY6dbeXyhMWn3zAj8a5Pq+/VkK3PdcEBVNADLXbh3a
lnDmjwsY9ZX64HhXbamFMV1Wykhbjb+Jprj6CJjuz4wcGArKW+lsTV86p8Q5Kzk=
=uYjg
-----END PGP SIGNATURE-----
[-- Attachment #2: xsa269.meta --]
[-- Type: application/octet-stream, Size: 1991 bytes --]
{
"XSA": 269,
"SupportedVersions": [
"master",
"4.11",
"4.10",
"4.9",
"4.8",
"4.7",
"4.6"
],
"Trees": [
"xen"
],
"Recipes": {
"4.10": {
"Recipes": {
"xen": {
"StableRef": "87c83af333e0248ada2e6560965aca6096ec7f2b",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.10.patch"
]
}
}
},
"4.11": {
"Recipes": {
"xen": {
"StableRef": "33ced725e11af4eabd3334d12f53ed807e9e2586",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.11.patch"
]
}
}
},
"4.6": {
"Recipes": {
"xen": {
"StableRef": "98d7948b50b4e91ec4efa860da32d9ac4fe69300",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.8.patch"
]
}
}
},
"4.7": {
"Recipes": {
"xen": {
"StableRef": "91ca84c862b15fe74ab9b5870e66903aec4f86dd",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.8.patch"
]
}
}
},
"4.8": {
"Recipes": {
"xen": {
"StableRef": "aa450153f2d960c217149b31b68a8b57c5a8e595",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.8.patch"
]
}
}
},
"4.9": {
"Recipes": {
"xen": {
"StableRef": "a1b223b756f354895525060bd3f9f1f07899a082",
"Prereqs": [
268
],
"Patches": [
"xsa269-4.10.patch"
]
}
}
},
"master": {
"Recipes": {
"xen": {
"StableRef": "acd00a303378ce48bd6bbd8a579f1fe2f1b21a7d",
"Prereqs": [
268
],
"Patches": [
"xsa269.patch"
]
}
}
}
}
}
[-- Attachment #3: xsa269.patch --]
[-- Type: application/octet-stream, Size: 4733 bytes --]
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL bits
The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s
11fe998e56 bypasses all reserved bit checking in the general case. As a
result, a guest can enable BTS when it shouldn't be permitted to, and
lock up the entire host.
With vPMU active (not a security supported configuration, but useful for
debugging), the reserved bit checking in broken, caused by the original
BTS changeset 1a8aa75ed.
From a correctness standpoint, it is not possible to have two different
pieces of code responsible for different parts of value checking, if
there isn't an accumulation of bits which have been checked. A
practical upshot of this is that a guest can set any value it
wishes (usually resulting in a vmentry failure for bad guest state).
Therefore, fix this by implementing all the reserved bit checking in the
main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the
vPMU MSR logic.
This is XSA-269
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 1fc79c9..6e27f6e 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -533,27 +533,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
uint64_t *enabled_cntrs;
if ( !core2_vpmu_msr_common_check(msr, &type, &index) )
- {
- /* Special handling for BTS */
- if ( msr == MSR_IA32_DEBUGCTLMSR )
- {
- supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
-
- if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
- supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
- IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( !(msr_content & ~supported) &&
- vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 0;
- if ( (msr_content & supported) &&
- !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- printk(XENLOG_G_WARNING
- "%pv: Debug Store unsupported on this CPU\n",
- current);
- }
return -EINVAL;
- }
ASSERT(!supported);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 12e0ee5..8da05f5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3081,11 +3081,14 @@ void vmx_vlapic_msr_changed(struct vcpu *v)
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
struct vcpu *v = current;
+ const struct cpuid_policy *cp = v->domain->arch.cpuid;
HVM_DBG_LOG(DBG_LEVEL_MSR, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
switch ( msr )
{
+ uint64_t rsvd;
+
case MSR_IA32_SYSENTER_CS:
__vmwrite(GUEST_SYSENTER_CS, msr_content);
break;
@@ -3138,18 +3141,26 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
wrmsrl(MSR_SYSCALL_MASK, msr_content);
break;
- case MSR_IA32_DEBUGCTLMSR: {
- uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
+ case MSR_IA32_DEBUGCTLMSR:
+ rsvd = ~(IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF);
- if ( boot_cpu_has(X86_FEATURE_RTM) )
- supported |= IA32_DEBUGCTLMSR_RTM;
- if ( msr_content & ~supported )
+ /* TODO: Wire vPMU settings properly through the CPUID policy */
+ if ( vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_BTS) )
{
- /* Perhaps some other bits are supported in vpmu. */
- if ( vpmu_do_wrmsr(msr, msr_content, supported) )
- break;
+ rsvd &= ~(IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT);
+
+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
+ rsvd &= ~(IA32_DEBUGCTLMSR_BTS_OFF_OS |
+ IA32_DEBUGCTLMSR_BTS_OFF_USR);
}
+ if ( cp->feat.rtm )
+ rsvd &= ~IA32_DEBUGCTLMSR_RTM;
+
+ if ( msr_content & rsvd )
+ goto gp_fault;
+
/*
* When a guest first enables LBR, arrange to save and restore the LBR
* MSRs and allow the guest direct access.
@@ -3208,7 +3219,7 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
__vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
break;
- }
+
case MSR_IA32_FEATURE_CONTROL:
case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
/* None of these MSRs are writeable. */
[-- Attachment #4: xsa269-4.8.patch --]
[-- Type: application/octet-stream, Size: 5351 bytes --]
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL bits
The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s
11fe998e56 bypasses all reserved bit checking in the general case. As a
result, a guest can enable BTS when it shouldn't be permitted to, and
lock up the entire host.
With vPMU active (not a security supported configuration, but useful for
debugging), the reserved bit checking in broken, caused by the original
BTS changeset 1a8aa75ed.
From a correctness standpoint, it is not possible to have two different
pieces of code responsible for different parts of value checking, if
there isn't an accumulation of bits which have been checked. A
practical upshot of this is that a guest can set any value it
wishes (usually resulting in a vmentry failure for bad guest state).
Therefore, fix this by implementing all the reserved bit checking in the
main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the
vPMU MSR logic.
This is XSA-269
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 7f9add1..6276c10 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -68,10 +68,6 @@
#define MSR_PMC_ALIAS_MASK (~(MSR_IA32_PERFCTR0 ^ MSR_IA32_A_PERFCTR0))
static bool_t __read_mostly full_width_write;
-/* Intel-specific VPMU features */
-#define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */
-#define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store */
-
/*
* MSR_CORE_PERF_FIXED_CTR_CTRL contains the configuration of all fixed
* counters. 4 bits for every counter.
@@ -563,27 +559,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
uint64_t *enabled_cntrs;
if ( !core2_vpmu_msr_common_check(msr, &type, &index) )
- {
- /* Special handling for BTS */
- if ( msr == MSR_IA32_DEBUGCTLMSR )
- {
- supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
-
- if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
- supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
- IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( !(msr_content & ~supported) &&
- vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 0;
- if ( (msr_content & supported) &&
- !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- printk(XENLOG_G_WARNING
- "%pv: Debug Store unsupported on this CPU\n",
- current);
- }
return -EINVAL;
- }
ASSERT(!supported);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 955643c..d07d941 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2976,6 +2976,8 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
switch ( msr )
{
+ uint64_t rsvd;
+
case MSR_IA32_SYSENTER_CS:
__vmwrite(GUEST_SYSENTER_CS, msr_content);
break;
@@ -2990,17 +2992,29 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
__vmwrite(GUEST_SYSENTER_EIP, msr_content);
break;
case MSR_IA32_DEBUGCTLMSR: {
+ uint32_t ebx, ecx = 0;
int i, rc = 0;
- uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
- if ( boot_cpu_has(X86_FEATURE_RTM) )
- supported |= IA32_DEBUGCTLMSR_RTM;
- if ( msr_content & ~supported )
+ rsvd = ~(IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF);
+
+ /* TODO: Wire vPMU settings properly through the CPUID policy */
+ if ( vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_BTS) )
{
- /* Perhaps some other bits are supported in vpmu. */
- if ( vpmu_do_wrmsr(msr, msr_content, supported) )
- break;
+ rsvd &= ~(IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT);
+
+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
+ rsvd &= ~(IA32_DEBUGCTLMSR_BTS_OFF_OS |
+ IA32_DEBUGCTLMSR_BTS_OFF_USR);
}
+
+ hvm_cpuid(7, NULL, &ebx, &ecx, NULL);
+ if ( ebx & cpufeat_mask(X86_FEATURE_RTM) )
+ rsvd &= ~IA32_DEBUGCTLMSR_RTM;
+
+ if ( msr_content & rsvd )
+ goto gp_fault;
+
if ( msr_content & IA32_DEBUGCTLMSR_LBR )
{
const struct lbr_info *lbr = last_branch_msr_get();
diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h
index ed9ec07..75b1973 100644
--- a/xen/include/asm-x86/vpmu.h
+++ b/xen/include/asm-x86/vpmu.h
@@ -77,6 +77,10 @@ struct vpmu_struct {
/* PV(H) guests: VPMU registers are accessed by guest from shared page */
#define VPMU_CACHED 0x40
+/* Intel-specific VPMU features */
+#define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */
+#define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store */
+
static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask)
{
vpmu->flags |= mask;
[-- Attachment #5: xsa269-4.10.patch --]
[-- Type: application/octet-stream, Size: 4662 bytes --]
From 0653a0598613ea9d7598ad8f9ace89ed7498410c Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Mon, 30 Jul 2018 11:01:28 +0100
Subject: [PATCH] x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL
bits
The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s
11fe998e56 bypasses all reserved bit checking in the general case. As a
result, a guest can enable BTS when it shouldn't be permitted to, and
lock up the entire host.
With vPMU active (not a security supported configuration, but useful for
debugging), the reserved bit checking in broken, caused by the original
BTS changeset 1a8aa75ed.
From a correctness standpoint, it is not possible to have two different
pieces of code responsible for different parts of value checking, if
there isn't an accumulation of bits which have been checked. A
practical upshot of this is that a guest can set any value it
wishes (usually resulting in a vmentry failure for bad guest state).
Therefore, fix this by implementing all the reserved bit checking in the
main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the
vPMU MSR logic.
This is XSA-269
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/cpu/vpmu_intel.c | 20 --------------------
xen/arch/x86/hvm/vmx/vmx.c | 27 ++++++++++++++++++++-------
2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 207e2e712c..d4444f0d94 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -535,27 +535,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
uint64_t *enabled_cntrs;
if ( !core2_vpmu_msr_common_check(msr, &type, &index) )
- {
- /* Special handling for BTS */
- if ( msr == MSR_IA32_DEBUGCTLMSR )
- {
- supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
-
- if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
- supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
- IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( !(msr_content & ~supported) &&
- vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 0;
- if ( (msr_content & supported) &&
- !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- printk(XENLOG_G_WARNING
- "%pv: Debug Store unsupported on this CPU\n",
- current);
- }
return -EINVAL;
- }
ASSERT(!supported);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 48d3c54e84..2c490cf62a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3097,11 +3097,14 @@ void vmx_vlapic_msr_changed(struct vcpu *v)
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
struct vcpu *v = current;
+ const struct cpuid_policy *cp = v->domain->arch.cpuid;
HVM_DBG_LOG(DBG_LEVEL_MSR, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
switch ( msr )
{
+ uint64_t rsvd;
+
case MSR_IA32_SYSENTER_CS:
__vmwrite(GUEST_SYSENTER_CS, msr_content);
break;
@@ -3117,16 +3120,26 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
break;
case MSR_IA32_DEBUGCTLMSR: {
int i, rc = 0;
- uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
- if ( boot_cpu_has(X86_FEATURE_RTM) )
- supported |= IA32_DEBUGCTLMSR_RTM;
- if ( msr_content & ~supported )
+ rsvd = ~(IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF);
+
+ /* TODO: Wire vPMU settings properly through the CPUID policy */
+ if ( vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_BTS) )
{
- /* Perhaps some other bits are supported in vpmu. */
- if ( vpmu_do_wrmsr(msr, msr_content, supported) )
- break;
+ rsvd &= ~(IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT);
+
+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
+ rsvd &= ~(IA32_DEBUGCTLMSR_BTS_OFF_OS |
+ IA32_DEBUGCTLMSR_BTS_OFF_USR);
}
+
+ if ( cp->feat.rtm )
+ rsvd &= ~IA32_DEBUGCTLMSR_RTM;
+
+ if ( msr_content & rsvd )
+ goto gp_fault;
+
if ( msr_content & IA32_DEBUGCTLMSR_LBR )
{
const struct lbr_info *lbr = last_branch_msr_get();
--
2.18.0
[-- Attachment #6: xsa269-4.11.patch --]
[-- Type: application/octet-stream, Size: 4329 bytes --]
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL bits
The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s
11fe998e56 bypasses all reserved bit checking in the general case. As a
result, a guest can enable BTS when it shouldn't be permitted to, and
lock up the entire host.
With vPMU active (not a security supported configuration, but useful for
debugging), the reserved bit checking in broken, caused by the original
BTS changeset 1a8aa75ed.
From a correctness standpoint, it is not possible to have two different
pieces of code responsible for different parts of value checking, if
there isn't an accumulation of bits which have been checked. A
practical upshot of this is that a guest can set any value it
wishes (usually resulting in a vmentry failure for bad guest state).
Therefore, fix this by implementing all the reserved bit checking in the
main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the
vPMU MSR logic.
This is XSA-269
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 207e2e7..d4444f0 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -535,27 +535,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
uint64_t *enabled_cntrs;
if ( !core2_vpmu_msr_common_check(msr, &type, &index) )
- {
- /* Special handling for BTS */
- if ( msr == MSR_IA32_DEBUGCTLMSR )
- {
- supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
-
- if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
- supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
- IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( !(msr_content & ~supported) &&
- vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 0;
- if ( (msr_content & supported) &&
- !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- printk(XENLOG_G_WARNING
- "%pv: Debug Store unsupported on this CPU\n",
- current);
- }
return -EINVAL;
- }
ASSERT(!supported);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9707514..ae028dd 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3032,11 +3032,14 @@ void vmx_vlapic_msr_changed(struct vcpu *v)
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
struct vcpu *v = current;
+ const struct cpuid_policy *cp = v->domain->arch.cpuid;
HVM_DBG_LOG(DBG_LEVEL_MSR, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
switch ( msr )
{
+ uint64_t rsvd;
+
case MSR_IA32_SYSENTER_CS:
__vmwrite(GUEST_SYSENTER_CS, msr_content);
break;
@@ -3091,16 +3094,26 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
case MSR_IA32_DEBUGCTLMSR: {
int i, rc = 0;
- uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
- if ( boot_cpu_has(X86_FEATURE_RTM) )
- supported |= IA32_DEBUGCTLMSR_RTM;
- if ( msr_content & ~supported )
+ rsvd = ~(IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF);
+
+ /* TODO: Wire vPMU settings properly through the CPUID policy */
+ if ( vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_BTS) )
{
- /* Perhaps some other bits are supported in vpmu. */
- if ( vpmu_do_wrmsr(msr, msr_content, supported) )
- break;
+ rsvd &= ~(IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT);
+
+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
+ rsvd &= ~(IA32_DEBUGCTLMSR_BTS_OFF_OS |
+ IA32_DEBUGCTLMSR_BTS_OFF_USR);
}
+
+ if ( cp->feat.rtm )
+ rsvd &= ~IA32_DEBUGCTLMSR_RTM;
+
+ if ( msr_content & rsvd )
+ goto gp_fault;
+
if ( msr_content & IA32_DEBUGCTLMSR_LBR )
{
const struct lbr_info *lbr = last_branch_msr_get();
[-- Attachment #7: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2018-08-20 9:47 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-20 9:47 Xen Security Advisory 269 v3 (CVE-2018-15468) - x86: Incorrect MSR_DEBUGCTL handling lets guests enable BTS Xen.org security team
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).