[Xen-devel] [PATCH v4 2/7] arch, arm: add consistency checks to REMOVE p2m changes

[Arianna Avanzini <avanzini.arianna@xxxxxxxxx>]

Currently, the REMOVE case of the switch in apply_p2m_changes()
does not perform any consistency check on the mapping to be removed.
More in detail, the code does not check if the guest address to be
unmapped is actually mapped to the machine address given as a
parameter.
This commit attempts to add the above-described consistency check
to the REMOVE path of apply_p2m_changes(). This is instrumental to
one of the following commits which implements the possibility to
trigger the removal of p2m ranges via the memory_mapping DOMCTL
for ARM.

Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>
Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>

---

    v4:
        - Remove useless and slow lookup and use already-available
          data from pte instead.
        - Correctly increment the local variable used to keep the
          machine address whose mapping is currently being removed.
        - Return with an error upon finding a mismatch between the
          actual machine address mapped to the guest address and
          the machine address passed as parameter, instead of just
          skipping the page.

---
 xen/arch/arm/p2m.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d00c882..bb0db16 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -243,12 +243,13 @@ static int apply_p2m_changes(struct domain *d,
     int rc;
     struct p2m_domain *p2m = &d->arch.p2m;
     lpae_t *first = NULL, *second = NULL, *third = NULL;
-    paddr_t addr;
+    paddr_t addr, _maddr;
     unsigned long cur_first_page = ~0,
                   cur_first_offset = ~0,
                   cur_second_offset = ~0;
     unsigned long count = 0;
     unsigned int flush = 0;
+    unsigned long mfn;
     bool_t populate = (op == INSERT || op == ALLOCATE);
     lpae_t pte;
 
@@ -258,6 +259,7 @@ static int apply_p2m_changes(struct domain *d,
         p2m_load_VTTBR(d);
 
     addr = start_gpaddr;
+    _maddr = maddr;
     while ( addr < end_gpaddr )
     {
         if ( cur_first_page != p2m_first_level_index(addr) )
@@ -327,6 +329,7 @@ static int apply_p2m_changes(struct domain *d,
 
         flush |= pte.p2m.valid;
 
+        mfn = pte.p2m.base;
         /* TODO: Handle other p2m type
          *
          * It's safe to do the put_page here because page_alloc will
@@ -335,8 +338,6 @@ static int apply_p2m_changes(struct domain *d,
          */
         if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) )
         {
-            unsigned long mfn = pte.p2m.base;
-
             ASSERT(mfn_valid(mfn));
             put_page(mfn_to_page(mfn));
         }
@@ -367,9 +368,23 @@ static int apply_p2m_changes(struct domain *d,
                     maddr += PAGE_SIZE;
                 }
                 break;
-            case RELINQUISH:
             case REMOVE:
                 {
+                    ASSERT(pte.p2m.valid);
+                    /*
+                     * Ensure that the guest address given as argument to
+                     * this function is actually mapped to the specified
+                     * machine address. _maddr here is the machine address
+                     * given to the function, while mfn is the machine
+                     * frame number actually mapped to the guest address:
+                     * check if the two correspond.
+                     */
+                    if ( _maddr != pfn_to_paddr(mfn) )
+                        return -EINVAL;
+                }
+                /* fall through */
+            case RELINQUISH:
+                {
                     if ( !pte.p2m.valid )
                     {
                         count++;
@@ -408,6 +423,7 @@ static int apply_p2m_changes(struct domain *d,
 
         /* Got the next page */
         addr += PAGE_SIZE;
+        _maddr += PAGE_SIZE;
     }
 
     if ( flush )
-- 
1.9.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel