Re: [Xen-devel] [PATCH for-4.13] x86/vvmx: Fix livelock with XSA-304 fix
On 23/11/2019 05:56, Jürgen Groß wrote: > On 22.11.19 18:54, Andrew Cooper wrote: >> It turns out that the XSA-304 / CVE-2018-12207 fix of disabling >> executable >> superpages doesn't work well with the nested p2m code. >> >> Nested virt is experimental and not security supported, but is useful >> for >> development purposes. In order to not regress the status quo, >> disable the >> XSA-304 workaround until the nested p2m code can be improved. >> >> Introduce a per-domain exec_sp control and set it based on the current >> opt_ept_exec_sp setting. Take the oppotunity to omit a PVH hardware >> domain >> from the performance hit, because it is already permitted to DoS the >> system in >> such ways as issuing a reboot. >> >> When nested virt is enabled on a domain, force it to using executable >> superpages and rebuild the p2m. >> >> Having the setting per-domain involves rearranging the internals of >> parse_ept_param_runtime() but it still retains the same overall >> semantics - >> for each applicable domain whose setting needs to change, rebuild the >> p2m. >> >> Signed-off-by: Andrew Cooper > > Release-acked-by: Juergen Gross Thanks. I've pushed this to staging to hopefully unblock OSSTest for RC3. Any further delta to this patch can be done incrementally. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] x86/vvmx: Fix livelock with XSA-304 fix
On 22.11.19 18:54, Andrew Cooper wrote: It turns out that the XSA-304 / CVE-2018-12207 fix of disabling executable superpages doesn't work well with the nested p2m code. Nested virt is experimental and not security supported, but is useful for development purposes. In order to not regress the status quo, disable the XSA-304 workaround until the nested p2m code can be improved. Introduce a per-domain exec_sp control and set it based on the current opt_ept_exec_sp setting. Take the oppotunity to omit a PVH hardware domain from the performance hit, because it is already permitted to DoS the system in such ways as issuing a reboot. When nested virt is enabled on a domain, force it to using executable superpages and rebuild the p2m. Having the setting per-domain involves rearranging the internals of parse_ept_param_runtime() but it still retains the same overall semantics - for each applicable domain whose setting needs to change, rebuild the p2m. Signed-off-by: Andrew Cooper Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] x86/vvmx: Fix livelock with XSA-304 fix
On 22/11/2019 18:08, George Dunlap wrote:
> On Fri, Nov 22, 2019 at 5:55 PM Andrew Cooper
> wrote:
>> It turns out that the XSA-304 / CVE-2018-12207 fix of disabling executable
>> superpages doesn't work well with the nested p2m code.
>>
>> Nested virt is experimental and not security supported, but is useful for
>> development purposes. In order to not regress the status quo, disable the
>> XSA-304 workaround until the nested p2m code can be improved.
>>
>> Introduce a per-domain exec_sp control and set it based on the current
>> opt_ept_exec_sp setting. Take the oppotunity to omit a PVH hardware domain
>> from the performance hit, because it is already permitted to DoS the system
>> in
>> such ways as issuing a reboot.
>>
>> When nested virt is enabled on a domain, force it to using executable
>> superpages and rebuild the p2m.
>>
>> Having the setting per-domain involves rearranging the internals of
>> parse_ept_param_runtime() but it still retains the same overall semantics -
>> for each applicable domain whose setting needs to change, rebuild the p2m.
>>
>> Signed-off-by: Andrew Cooper
>> ---
>> CC: Jan Beulich
>> CC: Wei Liu
>> CC: Roger Pau Monné
>> CC: Juergen Gross
>> ---
>> xen/arch/x86/hvm/vmx/vmcs.c| 31 +++
>> xen/arch/x86/hvm/vmx/vmx.c | 6 ++
>> xen/arch/x86/hvm/vmx/vvmx.c| 13 +
>> xen/arch/x86/mm/p2m-ept.c | 2 +-
>> xen/include/asm-x86/hvm/vmx/vmcs.h | 6 ++
>> 5 files changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
>> index 477c968409..f10f6b78ec 100644
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -31,6 +31,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -97,6 +98,7 @@ custom_param("ept", parse_ept_param);
>>
>> static int parse_ept_param_runtime(const char *s)
>> {
>> +struct domain *d;
>> int val;
>>
>> if ( !cpu_has_vmx_ept || !hvm_funcs.hap_supported ||
>> @@ -110,18 +112,31 @@ static int parse_ept_param_runtime(const char *s)
>> if ( (val = parse_boolean("exec-sp", s, NULL)) < 0 )
>> return -EINVAL;
>>
>> -if ( val != opt_ept_exec_sp )
>> +opt_ept_exec_sp = val;
>> +
>> +rcu_read_lock(_read_lock);
>> +for_each_domain ( d )
>> {
>> -struct domain *d;
>> +/* PV, or HVM Shadow domain? Not applicable. */
>> +if ( !paging_mode_hap(d) )
>> +continue;
>>
>> -opt_ept_exec_sp = val;
>> +/* Hardware domain? Not applicable. */
>> +if ( is_hardware_domain(d) )
>> +continue;
>>
>> -rcu_read_lock(_read_lock);
>> -for_each_domain ( d )
>> -if ( paging_mode_hap(d) )
>> -p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
>> -rcu_read_unlock(_read_lock);
>> +/* Nested Virt? Broken and exec_sp forced on to avoid livelocks. */
>> +if ( nestedhvm_enabled(d) )
>> +continue;
>> +
>> +/* Setting already matches? No need to rebuild the p2m. */
>> +if ( d->arch.hvm.vmx.exec_sp == val )
>> +continue;
>> +
>> +d->arch.hvm.vmx.exec_sp = val;
>> +p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
>> }
>> +rcu_read_unlock(_read_lock);
>>
>> printk("VMX: EPT executable superpages %sabled\n",
>> val ? "en" : "dis");
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 6a5eeb5c13..a71df71bc1 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -404,6 +404,12 @@ static int vmx_domain_initialise(struct domain *d)
>>
>> d->arch.ctxt_switch =
>>
>> +/*
>> + * Work around CVE-2018-12207? The hardware domain is already permitted
>> + * to reboot the system, so doesn't need mitigating against DoS's.
>> + */
>> +d->arch.hvm.vmx.exec_sp = is_hardware_domain(d) || opt_ept_exec_sp;
>> +
>> if ( !has_vlapic(d) )
>> return 0;
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> index 6696bd6240..5dd00e11b5 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -63,10 +63,23 @@ void nvmx_cpu_dead(unsigned int cpu)
>>
>> int nvmx_vcpu_initialise(struct vcpu *v)
>> {
>> +struct domain *d = v->domain;
>> struct nestedvmx *nvmx = _2_nvmx(v);
>> struct nestedvcpu *nvcpu = _nestedhvm(v);
>> struct page_info *pg = alloc_domheap_page(NULL, 0);
>>
>> +/*
>> + * Gross bodge. The nested p2m logic can't cope with the CVE-2018-12207
>> + * workaround of using NX EPT superpages, and livelocks. Nested HVM
>> isn't
>> + * security supported, so disable the workaround until the nested p2m
>> + * logic can be improved.
>> + */
>> +if ( !d->arch.hvm.vmx.exec_sp )
>> +{
>> +
Re: [Xen-devel] [PATCH for-4.13] x86/vvmx: Fix livelock with XSA-304 fix
On Fri, Nov 22, 2019 at 5:55 PM Andrew Cooper wrote:
>
> It turns out that the XSA-304 / CVE-2018-12207 fix of disabling executable
> superpages doesn't work well with the nested p2m code.
>
> Nested virt is experimental and not security supported, but is useful for
> development purposes. In order to not regress the status quo, disable the
> XSA-304 workaround until the nested p2m code can be improved.
>
> Introduce a per-domain exec_sp control and set it based on the current
> opt_ept_exec_sp setting. Take the oppotunity to omit a PVH hardware domain
> from the performance hit, because it is already permitted to DoS the system in
> such ways as issuing a reboot.
>
> When nested virt is enabled on a domain, force it to using executable
> superpages and rebuild the p2m.
>
> Having the setting per-domain involves rearranging the internals of
> parse_ept_param_runtime() but it still retains the same overall semantics -
> for each applicable domain whose setting needs to change, rebuild the p2m.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> CC: Juergen Gross
> ---
> xen/arch/x86/hvm/vmx/vmcs.c| 31 +++
> xen/arch/x86/hvm/vmx/vmx.c | 6 ++
> xen/arch/x86/hvm/vmx/vvmx.c| 13 +
> xen/arch/x86/mm/p2m-ept.c | 2 +-
> xen/include/asm-x86/hvm/vmx/vmcs.h | 6 ++
> 5 files changed, 49 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 477c968409..f10f6b78ec 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -97,6 +98,7 @@ custom_param("ept", parse_ept_param);
>
> static int parse_ept_param_runtime(const char *s)
> {
> +struct domain *d;
> int val;
>
> if ( !cpu_has_vmx_ept || !hvm_funcs.hap_supported ||
> @@ -110,18 +112,31 @@ static int parse_ept_param_runtime(const char *s)
> if ( (val = parse_boolean("exec-sp", s, NULL)) < 0 )
> return -EINVAL;
>
> -if ( val != opt_ept_exec_sp )
> +opt_ept_exec_sp = val;
> +
> +rcu_read_lock(_read_lock);
> +for_each_domain ( d )
> {
> -struct domain *d;
> +/* PV, or HVM Shadow domain? Not applicable. */
> +if ( !paging_mode_hap(d) )
> +continue;
>
> -opt_ept_exec_sp = val;
> +/* Hardware domain? Not applicable. */
> +if ( is_hardware_domain(d) )
> +continue;
>
> -rcu_read_lock(_read_lock);
> -for_each_domain ( d )
> -if ( paging_mode_hap(d) )
> -p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
> -rcu_read_unlock(_read_lock);
> +/* Nested Virt? Broken and exec_sp forced on to avoid livelocks. */
> +if ( nestedhvm_enabled(d) )
> +continue;
> +
> +/* Setting already matches? No need to rebuild the p2m. */
> +if ( d->arch.hvm.vmx.exec_sp == val )
> +continue;
> +
> +d->arch.hvm.vmx.exec_sp = val;
> +p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
> }
> +rcu_read_unlock(_read_lock);
>
> printk("VMX: EPT executable superpages %sabled\n",
> val ? "en" : "dis");
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 6a5eeb5c13..a71df71bc1 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -404,6 +404,12 @@ static int vmx_domain_initialise(struct domain *d)
>
> d->arch.ctxt_switch =
>
> +/*
> + * Work around CVE-2018-12207? The hardware domain is already permitted
> + * to reboot the system, so doesn't need mitigating against DoS's.
> + */
> +d->arch.hvm.vmx.exec_sp = is_hardware_domain(d) || opt_ept_exec_sp;
> +
> if ( !has_vlapic(d) )
> return 0;
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 6696bd6240..5dd00e11b5 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -63,10 +63,23 @@ void nvmx_cpu_dead(unsigned int cpu)
>
> int nvmx_vcpu_initialise(struct vcpu *v)
> {
> +struct domain *d = v->domain;
> struct nestedvmx *nvmx = _2_nvmx(v);
> struct nestedvcpu *nvcpu = _nestedhvm(v);
> struct page_info *pg = alloc_domheap_page(NULL, 0);
>
> +/*
> + * Gross bodge. The nested p2m logic can't cope with the CVE-2018-12207
> + * workaround of using NX EPT superpages, and livelocks. Nested HVM
> isn't
> + * security supported, so disable the workaround until the nested p2m
> + * logic can be improved.
> + */
> +if ( !d->arch.hvm.vmx.exec_sp )
> +{
> +d->arch.hvm.vmx.exec_sp = true;
> +p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
There wasn't an issue with nested guests having to deal with the
changed entry type?
Assuming
[Xen-devel] [PATCH for-4.13] x86/vvmx: Fix livelock with XSA-304 fix
It turns out that the XSA-304 / CVE-2018-12207 fix of disabling executable
superpages doesn't work well with the nested p2m code.
Nested virt is experimental and not security supported, but is useful for
development purposes. In order to not regress the status quo, disable the
XSA-304 workaround until the nested p2m code can be improved.
Introduce a per-domain exec_sp control and set it based on the current
opt_ept_exec_sp setting. Take the oppotunity to omit a PVH hardware domain
from the performance hit, because it is already permitted to DoS the system in
such ways as issuing a reboot.
When nested virt is enabled on a domain, force it to using executable
superpages and rebuild the p2m.
Having the setting per-domain involves rearranging the internals of
parse_ept_param_runtime() but it still retains the same overall semantics -
for each applicable domain whose setting needs to change, rebuild the p2m.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Juergen Gross
---
xen/arch/x86/hvm/vmx/vmcs.c| 31 +++
xen/arch/x86/hvm/vmx/vmx.c | 6 ++
xen/arch/x86/hvm/vmx/vvmx.c| 13 +
xen/arch/x86/mm/p2m-ept.c | 2 +-
xen/include/asm-x86/hvm/vmx/vmcs.h | 6 ++
5 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 477c968409..f10f6b78ec 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -97,6 +98,7 @@ custom_param("ept", parse_ept_param);
static int parse_ept_param_runtime(const char *s)
{
+struct domain *d;
int val;
if ( !cpu_has_vmx_ept || !hvm_funcs.hap_supported ||
@@ -110,18 +112,31 @@ static int parse_ept_param_runtime(const char *s)
if ( (val = parse_boolean("exec-sp", s, NULL)) < 0 )
return -EINVAL;
-if ( val != opt_ept_exec_sp )
+opt_ept_exec_sp = val;
+
+rcu_read_lock(_read_lock);
+for_each_domain ( d )
{
-struct domain *d;
+/* PV, or HVM Shadow domain? Not applicable. */
+if ( !paging_mode_hap(d) )
+continue;
-opt_ept_exec_sp = val;
+/* Hardware domain? Not applicable. */
+if ( is_hardware_domain(d) )
+continue;
-rcu_read_lock(_read_lock);
-for_each_domain ( d )
-if ( paging_mode_hap(d) )
-p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
-rcu_read_unlock(_read_lock);
+/* Nested Virt? Broken and exec_sp forced on to avoid livelocks. */
+if ( nestedhvm_enabled(d) )
+continue;
+
+/* Setting already matches? No need to rebuild the p2m. */
+if ( d->arch.hvm.vmx.exec_sp == val )
+continue;
+
+d->arch.hvm.vmx.exec_sp = val;
+p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
}
+rcu_read_unlock(_read_lock);
printk("VMX: EPT executable superpages %sabled\n",
val ? "en" : "dis");
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 6a5eeb5c13..a71df71bc1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -404,6 +404,12 @@ static int vmx_domain_initialise(struct domain *d)
d->arch.ctxt_switch =
+/*
+ * Work around CVE-2018-12207? The hardware domain is already permitted
+ * to reboot the system, so doesn't need mitigating against DoS's.
+ */
+d->arch.hvm.vmx.exec_sp = is_hardware_domain(d) || opt_ept_exec_sp;
+
if ( !has_vlapic(d) )
return 0;
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 6696bd6240..5dd00e11b5 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -63,10 +63,23 @@ void nvmx_cpu_dead(unsigned int cpu)
int nvmx_vcpu_initialise(struct vcpu *v)
{
+struct domain *d = v->domain;
struct nestedvmx *nvmx = _2_nvmx(v);
struct nestedvcpu *nvcpu = _nestedhvm(v);
struct page_info *pg = alloc_domheap_page(NULL, 0);
+/*
+ * Gross bodge. The nested p2m logic can't cope with the CVE-2018-12207
+ * workaround of using NX EPT superpages, and livelocks. Nested HVM isn't
+ * security supported, so disable the workaround until the nested p2m
+ * logic can be improved.
+ */
+if ( !d->arch.hvm.vmx.exec_sp )
+{
+d->arch.hvm.vmx.exec_sp = true;
+p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_rw);
+}
+
if ( !pg )
{
gdprintk(XENLOG_ERR, "nest: allocation for shadow vmcs failed\n");
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f06e51904a..b5517769c9 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -178,7 +178,7 @@ static void ept_p2m_type_to_flags(struct p2m_domain *p2m,
ept_entry_t *entry,
* Don't create
