[Xen-devel] [PATCH V6 1/4] x86/mm: Add array_index_nospec to guest provided index values

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

This patch aims to sanitize indexes, potentially guest provided
values, for altp2m_eptp[] and altp2m_p2m[] arrays.

Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
---
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
CC: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
---
Changes since V5:
        - Add black lines
        - Check altp2m_idx against min(ARRAY_SIZE(d->arch.altp2m_p2m),
MAX_EPTP).
---
 xen/arch/x86/mm/mem_access.c | 21 ++++++++++++---------
 xen/arch/x86/mm/p2m.c        | 26 ++++++++++++++++++--------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 320b9fe621..a95a50bcae 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -366,11 +366,12 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
uint32_t nr,
 #ifdef CONFIG_HVM
     if ( altp2m_idx )
     {
-        if ( altp2m_idx >= MAX_ALTP2M ||
-             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+             mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        ap2m = d->arch.altp2m_p2m[altp2m_idx];
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, MAX_ALTP2M)];
     }
 #else
     ASSERT(!altp2m_idx);
@@ -425,11 +426,12 @@ long p2m_set_mem_access_multi(struct domain *d,
 #ifdef CONFIG_HVM
     if ( altp2m_idx )
     {
-        if ( altp2m_idx >= MAX_ALTP2M ||
-             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+             mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        ap2m = d->arch.altp2m_p2m[altp2m_idx];
+        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, MAX_ALTP2M)];
     }
 #else
     ASSERT(!altp2m_idx);
@@ -491,11 +493,12 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn, 
xenmem_access_t *access,
     }
     else if ( altp2m_idx ) /* altp2m view 0 is treated as the hostp2m */
     {
-        if ( altp2m_idx >= MAX_ALTP2M ||
-             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+             mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = d->arch.altp2m_p2m[altp2m_idx];
+        p2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, MAX_ALTP2M)];
     }
 #else
     ASSERT(!altp2m_idx);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 3119269073..4fc919a9c5 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2577,6 +2577,8 @@ int p2m_init_altp2m_by_id(struct domain *d, unsigned int 
idx)
     if ( idx >= MAX_ALTP2M )
         return rc;
 
+    idx = array_index_nospec(idx, MAX_ALTP2M);
+
     altp2m_list_lock(d);
 
     if ( d->arch.altp2m_eptp[idx] == mfn_x(INVALID_MFN) )
@@ -2618,6 +2620,8 @@ int p2m_destroy_altp2m_by_id(struct domain *d, unsigned 
int idx)
     if ( !idx || idx >= MAX_ALTP2M )
         return rc;
 
+    idx = array_index_nospec(idx, MAX_ALTP2M);
+
     rc = domain_pause_except_self(d);
     if ( rc )
         return rc;
@@ -2689,11 +2693,13 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned 
int idx,
     mfn_t mfn;
     int rc = -EINVAL;
 
-    if ( idx >= MAX_ALTP2M || d->arch.altp2m_eptp[idx] == mfn_x(INVALID_MFN) )
+    if ( idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+         d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
+         mfn_x(INVALID_MFN) )
         return rc;
 
     hp2m = p2m_get_hostp2m(d);
-    ap2m = d->arch.altp2m_p2m[idx];
+    ap2m = d->arch.altp2m_p2m[array_index_nospec(idx, MAX_ALTP2M)];
 
     p2m_lock(hp2m);
     p2m_lock(ap2m);
@@ -3032,11 +3038,13 @@ int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, 
bool suppress_ve,
 
     if ( altp2m_idx > 0 )
     {
-        if ( altp2m_idx >= MAX_ALTP2M ||
-             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+             mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = ap2m = d->arch.altp2m_p2m[altp2m_idx];
+        p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx,
+                                                           MAX_ALTP2M)];
     }
     else
         p2m = host_p2m;
@@ -3075,11 +3083,13 @@ int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, 
bool *suppress_ve,
 
     if ( altp2m_idx > 0 )
     {
-        if ( altp2m_idx >= MAX_ALTP2M ||
-             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
+        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+             mfn_x(INVALID_MFN) )
             return -EINVAL;
 
-        p2m = ap2m = d->arch.altp2m_p2m[altp2m_idx];
+        p2m = ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx,
+                                                           MAX_ALTP2M)];
     }
     else
         p2m = host_p2m;
-- 
2.17.1

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