[Xen-devel] [PATCH SpectreV1+L1TF v4 04/11] x86/hvm: block speculative out-of-bound accesses

[Norbert Manthey <nmanthey@xxxxxxxxx>]

There are multiple arrays in the HVM interface that are accessed
with indices that are provided by the guest. To avoid speculative
out-of-bound accesses, we use the array_index_nospec macro.

When blocking speculative out-of-bound accesses, we can classify arrays
into dynamic arrays and static arrays. Where the former are allocated
during run time, the size of the latter is known during compile time.
On static arrays, compiler might be able to block speculative accesses
in the future.

We introduce another macro that uses the ARRAY_SIZE macro to block
speculative accesses. For arrays that are statically accessed, this macro
can be used instead of the usual macro. Using this macro results in more
readable code, and allows to modify the way this case is handled in a
single place.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Reported-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>

---
 xen/arch/x86/hvm/hvm.c   | 27 ++++++++++++++++++++++-----
 xen/include/xen/nospec.h |  6 ++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@
 #include <xen/monitor.h>
 #include <xen/warning.h>
 #include <xen/vpci.h>
+#include <xen/nospec.h>
 #include <asm/shadow.h>
 #include <asm/hap.h>
 #include <asm/current.h>
@@ -2102,7 +2103,7 @@ int hvm_mov_from_cr(unsigned int cr, unsigned int gpr)
     case 2:
     case 3:
     case 4:
-        val = curr->arch.hvm.guest_cr[cr];
+        val = array_access_nospec(curr->arch.hvm.guest_cr, cr);
         break;
     case 8:
         val = (vlapic_get_reg(vcpu_vlapic(curr), APIC_TASKPRI) & 0xf0) >> 4;
@@ -3448,13 +3449,15 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
*msr_content)
         if ( !d->arch.cpuid->basic.mtrr )
             goto gp_fault;
         index = msr - MSR_MTRRfix16K_80000;
-        *msr_content = fixed_range_base[index + 1];
+        *msr_content = fixed_range_base[array_index_nospec(index + 1,
+                                   ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
         break;
     case MSR_MTRRfix4K_C0000...MSR_MTRRfix4K_F8000:
         if ( !d->arch.cpuid->basic.mtrr )
             goto gp_fault;
         index = msr - MSR_MTRRfix4K_C0000;
-        *msr_content = fixed_range_base[index + 3];
+        *msr_content = fixed_range_base[array_index_nospec(index + 3,
+                                   ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
         break;
     case MSR_IA32_MTRR_PHYSBASE(0)...MSR_IA32_MTRR_PHYSMASK(MTRR_VCNT_MAX - 1):
         if ( !d->arch.cpuid->basic.mtrr )
@@ -3463,7 +3466,8 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
*msr_content)
         if ( (index / 2) >=
              MASK_EXTR(v->arch.hvm.mtrr.mtrr_cap, MTRRcap_VCNT) )
             goto gp_fault;
-        *msr_content = var_range_base[index];
+        *msr_content = var_range_base[array_index_nospec(index,
+                          MASK_EXTR(v->arch.hvm.mtrr.mtrr_cap, MTRRcap_VCNT))];
         break;
 
     case MSR_IA32_XSS:
@@ -4026,7 +4030,8 @@ static int hvmop_set_evtchn_upcall_vector(
     if ( op.vector < 0x10 )
         return -EINVAL;
 
-    if ( op.vcpu >= d->max_vcpus || (v = d->vcpu[op.vcpu]) == NULL )
+    if ( op.vcpu >= d->max_vcpus ||
+        (v = d->vcpu[array_index_nospec(op.vcpu, d->max_vcpus)]) == NULL )
         return -ENOENT;
 
     printk(XENLOG_G_INFO "%pv: upcall vector %02x\n", v, op.vector);
@@ -4114,6 +4119,12 @@ static int hvmop_set_param(
     if ( a.index >= HVM_NR_PARAMS )
         return -EINVAL;
 
+    /*
+     * Make sure the guest controlled value a.index is bounded even during
+     * speculative execution.
+     */
+    a.index = array_index_nospec(a.index, HVM_NR_PARAMS);
+
     d = rcu_lock_domain_by_any_id(a.domid);
     if ( d == NULL )
         return -ESRCH;
@@ -4380,6 +4391,12 @@ static int hvmop_get_param(
     if ( a.index >= HVM_NR_PARAMS )
         return -EINVAL;
 
+    /*
+     * Make sure the guest controlled value a.index is bounded even during
+     * speculative execution.
+     */
+    a.index = array_index_nospec(a.index, HVM_NR_PARAMS);
+
     d = rcu_lock_domain_by_any_id(a.domid);
     if ( d == NULL )
         return -ESRCH;
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -59,6 +59,12 @@ static inline unsigned long array_index_mask_nospec(unsigned 
long index,
 })
 
 /*
+ * array_access_nospec - allow nospec access for static size arrays
+ */
+#define array_access_nospec(array, index)                               \
+    (array)[array_index_nospec(index, ARRAY_SIZE(array))]
+
+/*
  * allow to insert a read memory barrier into conditionals
  */
 #ifdef CONFIG_X86
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel