[Xen-devel] [PATCH v3 2/5] 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 that the type of the entry
is correct in case of I/O memory mapping removal; also, 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 checks
to the REMOVE path of apply_p2m_changes(). This is instrumental to
the following commit 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>
---
 xen/arch/arm/p2m.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d00c882..47bf154 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -243,7 +243,8 @@ 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;
+    p2m_type_t _t;
+    paddr_t addr, _maddr = INVALID_PADDR;
     unsigned long cur_first_page = ~0,
                   cur_first_offset = ~0,
                   cur_second_offset = ~0;
@@ -252,6 +253,20 @@ static int apply_p2m_changes(struct domain *d,
     bool_t populate = (op == INSERT || op == ALLOCATE);
     lpae_t pte;
 
+    /*
+     * As of now, the lookup is needed only in in case
+     * of REMOVE operation, as a consistency check on
+     * the existence of a mapping between the machine
+     * address and the start guest address given as
+     * parameters.
+     */
+    if (op == REMOVE)
+        /*
+         * Be sure to lookup before grabbing the p2m_lock,
+         * as the p2m_lookup() function holds it too.
+         */
+        _maddr = p2m_lookup(d, start_gpaddr, &_t);
+
     spin_lock(&p2m->lock);
 
     if ( d != current->domain )
@@ -367,9 +382,23 @@ static int apply_p2m_changes(struct domain *d,
                     maddr += PAGE_SIZE;
                 }
                 break;
-            case RELINQUISH:
             case REMOVE:
                 {
+                    /*
+                     * Ensure that, if we are trying to unmap I/O memory
+                     * ranges, the given gfn is p2m_mmio_direct.
+                     */
+                    if ( t == p2m_mmio_direct ? _t != p2m_mmio_direct : 0 ||
+                         paddr_to_pfn(_maddr) == INVALID_MFN ||
+                         maddr != _maddr )
+                    {
+                        count++;
+                        break;
+                    }
+                }
+                /* fall through */
+            case RELINQUISH:
+                {
                     if ( !pte.p2m.valid )
                     {
                         count++;
-- 
1.9.0


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