[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 20/22] arch/x86: use XSM hooks for get_pg_owner access checks



There are three callers of get_pg_owner:
 * do_mmuext_op, which does not have XSM hooks on all subfunctions
 * do_mmu_update, which has hooks that are inefficient
 * do_update_va_mapping_otherdomain, which has a simple XSM hook

In order to preserve return values for the do_mmuext_op hypercall, an
additional XSM hook is required to check the operation even for those
subfunctions that do not use the pg_owner field. This also covers the
MMUEXT_UNPIN_TABLE operation which did previously have an XSM hook.

The XSM hooks in do_mmu_update were capable of replacing the checks in
get_pg_owner; however, the hooks are buried in the inner loop of the
function - not very good for performance when XSM is enabled and these
turn in to indirect function calls. This patch removes the PTE from the
hooks and replaces it with a bitfield describing what accesses are being
requested. The XSM hook can then be called only when additional bits are
set instead of once per iteration of the loop.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
---
 tools/flask/policy/policy/flask/access_vectors |  1 +
 tools/flask/policy/policy/modules/xen/xen.if   |  4 +-
 xen/arch/x86/mm.c                              | 53 +++++++++++--------
 xen/include/xsm/dummy.h                        | 15 ++++--
 xen/include/xsm/xsm.h                          | 25 +++++----
 xen/xsm/dummy.c                                |  4 +-
 xen/xsm/flask/hooks.c                          | 71 +++++++++-----------------
 xen/xsm/flask/include/av_perm_to_string.h      |  1 +
 xen/xsm/flask/include/av_permissions.h         |  1 +
 9 files changed, 88 insertions(+), 87 deletions(-)

diff --git a/tools/flask/policy/policy/flask/access_vectors 
b/tools/flask/policy/policy/flask/access_vectors
index 45ac437..8324725 100644
--- a/tools/flask/policy/policy/flask/access_vectors
+++ b/tools/flask/policy/policy/flask/access_vectors
@@ -141,6 +141,7 @@ class mmu
     mfnlist
     memorymap
     remote_remap
+       mmuext_op
 }
 
 class shadow
diff --git a/tools/flask/policy/policy/modules/xen/xen.if 
b/tools/flask/policy/policy/modules/xen/xen.if
index d630f47..725d7a1 100644
--- a/tools/flask/policy/policy/modules/xen/xen.if
+++ b/tools/flask/policy/policy/modules/xen/xen.if
@@ -7,7 +7,7 @@
 
################################################################################
 define(`declare_domain_common', `
        allow $1 $2:grant { query setup };
-       allow $1 $2:mmu { adjust physmap map_read map_write stat pinpage 
updatemp };
+       allow $1 $2:mmu { adjust physmap map_read map_write stat pinpage 
updatemp mmuext_op };
        allow $1 $2:hvm { getparam setparam };
 ')
 
@@ -51,7 +51,7 @@ define(`create_domain_common', `
        allow $1 $2:domain2 { set_cpuid settsc };
        allow $1 $2:security check_context;
        allow $1 $2:shadow enable;
-       allow $1 $2:mmu {map_read map_write adjust memorymap physmap pinpage};
+       allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage 
mmuext_op };
        allow $1 $2:grant setup;
        allow $1 $2:hvm { cacheattr getparam hvmctl irqlevel pciroute sethvmc 
setparam pcilevel trackdirtyvram };
 ')
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c0c2bf3..04a056f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2619,11 +2619,6 @@ static struct domain *get_pg_owner(domid_t domid)
         pg_owner = rcu_lock_domain(dom_io);
         break;
     case DOMID_XEN:
-        if ( !IS_PRIV(curr) )
-        {
-            MEM_LOG("Cannot set foreign dom");
-            break;
-        }
         pg_owner = rcu_lock_domain(dom_xen);
         break;
     default:
@@ -2632,12 +2627,6 @@ static struct domain *get_pg_owner(domid_t domid)
             MEM_LOG("Unknown domain '%u'", domid);
             break;
         }
-        if ( !IS_PRIV_FOR(curr, pg_owner) )
-        {
-            MEM_LOG("Cannot set foreign dom");
-            rcu_unlock_domain(pg_owner);
-            pg_owner = NULL;
-        }
         break;
     }
 
@@ -2725,6 +2714,13 @@ long do_mmuext_op(
         goto out;
     }
 
+    rc = xsm_mmuext_op(d, pg_owner);
+    if ( rc )
+    {
+        rcu_unlock_domain(pg_owner);
+        goto out;
+    }
+
     for ( i = 0; i < count; i++ )
     {
         if ( hypercall_preempt_check() )
@@ -3164,6 +3160,8 @@ long do_mmu_update(
     struct vcpu *v = current;
     struct domain *d = v->domain, *pt_owner = d, *pg_owner;
     struct domain_mmap_cache mapcache;
+    uint32_t xsm_needed = 0;
+    uint32_t xsm_checked = 0;
 
     if ( unlikely(count & MMU_UPDATE_PREEMPTED) )
     {
@@ -3195,11 +3193,6 @@ long do_mmu_update(
             rc = -EINVAL;
             goto out;
         }
-        if ( !IS_PRIV_FOR(d, pt_owner) )
-        {
-            rc = -ESRCH;
-            goto out;
-        }
     }
 
     if ( (pg_owner = get_pg_owner((uint16_t)foreigndom)) == NULL )
@@ -3239,9 +3232,20 @@ long do_mmu_update(
         {
             p2m_type_t p2mt;
 
-            rc = xsm_mmu_normal_update(d, pt_owner, pg_owner, req.val);
-            if ( rc )
-                break;
+            xsm_needed |= XSM_MMU_NORMAL_UPDATE;
+            if ( get_pte_flags(req.val) & _PAGE_PRESENT )
+            {
+                xsm_needed |= XSM_MMU_UPDATE_READ;
+                if ( get_pte_flags(req.val) & _PAGE_RW )
+                    xsm_needed |= XSM_MMU_UPDATE_WRITE;
+            }
+            if ( xsm_needed != xsm_checked )
+            {
+                rc = xsm_mmu_update(d, pt_owner, pg_owner, xsm_needed);
+                if ( rc )
+                    break;
+                xsm_checked = xsm_needed;
+            }
             rc = -EINVAL;
 
             req.ptr -= cmd;
@@ -3353,9 +3357,14 @@ long do_mmu_update(
             mfn = req.ptr >> PAGE_SHIFT;
             gpfn = req.val;
 
-            rc = xsm_mmu_machphys_update(d, pg_owner, mfn);
-            if ( rc )
-                break;
+            xsm_needed |= XSM_MMU_MACHPHYS_UPDATE;
+            if ( xsm_needed != xsm_checked )
+            {
+                rc = xsm_mmu_update(d, pt_owner, pg_owner, xsm_needed);
+                if ( rc )
+                    break;
+                xsm_checked = xsm_needed;
+            }
 
             if ( unlikely(!get_page_from_pagenr(mfn, pg_owner)) )
             {
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 1760cd9..9c13d5c037 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -662,21 +662,28 @@ static XSM_DEFAULT(int, domain_memory_map) (struct domain 
*d)
     return 0;
 }
 
-static XSM_DEFAULT(int, mmu_normal_update) (struct domain *d, struct domain *t,
-                                            struct domain *f, intpte_t fpte)
+static XSM_DEFAULT(int, mmu_update) (struct domain *d, struct domain *t,
+                                     struct domain *f, uint32_t flags)
 {
+    if ( d != t && !IS_PRIV_FOR(d, t) )
+        return -EPERM;
+    if ( d != f && !IS_PRIV_FOR(d, f) )
+        return -EPERM;
     return 0;
 }
 
-static XSM_DEFAULT(int, mmu_machphys_update) (struct domain *d, struct domain 
*f,
-                                              unsigned long mfn)
+static XSM_DEFAULT(int, mmuext_op) (struct domain *d, struct domain *f)
 {
+    if ( d != f && !IS_PRIV_FOR(d, f) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_DEFAULT(int, update_va_mapping) (struct domain *d, struct domain 
*f, 
                                                             l1_pgentry_t pte)
 {
+    if ( d != f && !IS_PRIV_FOR(d, f) )
+        return -EPERM;
     return 0;
 }
 
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index c2c1efa..6d970b1 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -27,8 +27,6 @@ typedef u32 xsm_magic_t;
 #define XSM_MAGIC 0x00000000
 #endif
 
-#ifdef XSM_ENABLE
-
 extern char *policy_buffer;
 extern u32 policy_size;
 
@@ -170,9 +168,13 @@ struct xsm_operations {
     int (*getidletime) (void);
     int (*machine_memory_map) (void);
     int (*domain_memory_map) (struct domain *d);
-    int (*mmu_normal_update) (struct domain *d, struct domain *t,
-                              struct domain *f, intpte_t fpte);
-    int (*mmu_machphys_update) (struct domain *d1, struct domain *d2, unsigned 
long mfn);
+#define XSM_MMU_UPDATE_READ      1
+#define XSM_MMU_UPDATE_WRITE     2
+#define XSM_MMU_NORMAL_UPDATE    4
+#define XSM_MMU_MACHPHYS_UPDATE  8
+    int (*mmu_update) (struct domain *d, struct domain *t,
+                       struct domain *f, uint32_t flags);
+    int (*mmuext_op) (struct domain *d, struct domain *f);
     int (*update_va_mapping) (struct domain *d, struct domain *f, l1_pgentry_t 
pte);
     int (*add_to_physmap) (struct domain *d1, struct domain *d2);
     int (*sendtrigger) (struct domain *d);
@@ -186,6 +188,8 @@ struct xsm_operations {
 #endif
 };
 
+#ifdef XSM_ENABLE
+
 extern struct xsm_operations *xsm_ops;
 
 static inline void xsm_security_domaininfo (struct domain *d,
@@ -761,16 +765,15 @@ static inline int xsm_domain_memory_map(struct domain *d)
     return xsm_ops->domain_memory_map(d);
 }
 
-static inline int xsm_mmu_normal_update (struct domain *d, struct domain *t,
-                                         struct domain *f, intpte_t fpte)
+static inline int xsm_mmu_update (struct domain *d, struct domain *t,
+                                  struct domain *f, uint32_t flags)
 {
-    return xsm_ops->mmu_normal_update(d, t, f, fpte);
+    return xsm_ops->mmu_update(d, t, f, flags);
 }
 
-static inline int xsm_mmu_machphys_update (struct domain *d1, struct domain 
*d2,
-                                           unsigned long mfn)
+static inline int xsm_mmuext_op (struct domain *d, struct domain *f)
 {
-    return xsm_ops->mmu_machphys_update(d1, d2, mfn);
+    return xsm_ops->mmuext_op(d, f);
 }
 
 static inline int xsm_update_va_mapping(struct domain *d, struct domain *f, 
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index c82c464..9476be6 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -154,8 +154,8 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, getidletime);
     set_to_dummy_if_null(ops, machine_memory_map);
     set_to_dummy_if_null(ops, domain_memory_map);
-    set_to_dummy_if_null(ops, mmu_normal_update);
-    set_to_dummy_if_null(ops, mmu_machphys_update);
+    set_to_dummy_if_null(ops, mmu_update);
+    set_to_dummy_if_null(ops, mmuext_op);
     set_to_dummy_if_null(ops, update_va_mapping);
     set_to_dummy_if_null(ops, add_to_physmap);
     set_to_dummy_if_null(ops, remove_from_physmap);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f993696..bd2d792 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1432,65 +1432,44 @@ static int flask_domain_memory_map(struct domain *d)
     return current_has_perm(d, SECCLASS_MMU, MMU__MEMORYMAP);
 }
 
-static int domain_memory_perm(struct domain *d, struct domain *f, l1_pgentry_t 
pte)
+static int flask_mmu_update(struct domain *d, struct domain *t,
+                            struct domain *f, uint32_t flags)
 {
     int rc = 0;
-    u32 map_perms = MMU__MAP_READ;
-    unsigned long fgfn, fmfn;
-    p2m_type_t p2mt;
-
-    if ( !(l1e_get_flags(pte) & _PAGE_PRESENT) )
-        return 0;
-
-    if ( l1e_get_flags(pte) & _PAGE_RW )
-        map_perms |= MMU__MAP_WRITE;
-
-    fgfn = l1e_get_pfn(pte);
-    fmfn = mfn_x(get_gfn_query(f, fgfn, &p2mt));
-    put_gfn(f, fgfn);
+    u32 map_perms = 0;
 
-    if ( f->domain_id == DOMID_IO || !mfn_valid(fmfn) )
-    {
-        struct avc_audit_data ad;
-        u32 dsid, fsid;
-        rc = security_iomem_sid(fmfn, &fsid);
-        if ( rc )
-            return rc;
-        AVC_AUDIT_DATA_INIT(&ad, MEMORY);
-        ad.sdom = d;
-        ad.tdom = f;
-        ad.memory.pte = pte.l1;
-        ad.memory.mfn = fmfn;
-        dsid = domain_sid(d);
-        return avc_has_perm(dsid, fsid, SECCLASS_MMU, map_perms, &ad);
-    }
-
-    return domain_has_perm(d, f, SECCLASS_MMU, map_perms);
-}
-
-static int flask_mmu_normal_update(struct domain *d, struct domain *t,
-                                   struct domain *f, intpte_t fpte)
-{
-    int rc = 0;
-
-    if (d != t)
+    if ( (flags & XSM_MMU_NORMAL_UPDATE) && d != t )
         rc = domain_has_perm(d, t, SECCLASS_MMU, MMU__REMOTE_REMAP);
     if ( rc )
         return rc;
 
-    return domain_memory_perm(d, f, l1e_from_intpte(fpte));
+    if ( flags & XSM_MMU_UPDATE_READ )
+        map_perms |= MMU__MAP_READ;
+    if ( flags & XSM_MMU_UPDATE_WRITE )
+        map_perms |= MMU__MAP_WRITE;
+    if ( flags & XSM_MMU_MACHPHYS_UPDATE )
+        map_perms |= MMU__UPDATEMP;
+
+    if ( map_perms )
+        rc = domain_has_perm(d, f, SECCLASS_MMU, map_perms);
+    return rc;
 }
 
-static int flask_mmu_machphys_update(struct domain *d1, struct domain *d2,
-                                     unsigned long mfn)
+static int flask_mmuext_op(struct domain *d, struct domain *f)
 {
-    return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__UPDATEMP);
+    return domain_has_perm(d, f, SECCLASS_MMU, MMU__MMUEXT_OP);
 }
 
 static int flask_update_va_mapping(struct domain *d, struct domain *f,
                                    l1_pgentry_t pte)
 {
-    return domain_memory_perm(d, f, pte);
+    u32 map_perms = MMU__MAP_READ;
+    if ( !(l1e_get_flags(pte) & _PAGE_PRESENT) )
+        return 0;
+    if ( l1e_get_flags(pte) & _PAGE_RW )
+        map_perms |= MMU__MAP_WRITE;
+
+    return domain_has_perm(d, f, SECCLASS_MMU, map_perms);
 }
 
 static int flask_add_to_physmap(struct domain *d1, struct domain *d2)
@@ -1771,8 +1750,8 @@ static struct xsm_operations flask_ops = {
     .getidletime = flask_getidletime,
     .machine_memory_map = flask_machine_memory_map,
     .domain_memory_map = flask_domain_memory_map,
-    .mmu_normal_update = flask_mmu_normal_update,
-    .mmu_machphys_update = flask_mmu_machphys_update,
+    .mmu_update = flask_mmu_update,
+    .mmuext_op = flask_mmuext_op,
     .update_va_mapping = flask_update_va_mapping,
     .add_to_physmap = flask_add_to_physmap,
     .remove_from_physmap = flask_remove_from_physmap,
diff --git a/xen/xsm/flask/include/av_perm_to_string.h 
b/xen/xsm/flask/include/av_perm_to_string.h
index 186f1fa..8f65b96 100644
--- a/xen/xsm/flask/include/av_perm_to_string.h
+++ b/xen/xsm/flask/include/av_perm_to_string.h
@@ -111,6 +111,7 @@
    S_(SECCLASS_MMU, MMU__MFNLIST, "mfnlist")
    S_(SECCLASS_MMU, MMU__MEMORYMAP, "memorymap")
    S_(SECCLASS_MMU, MMU__REMOTE_REMAP, "remote_remap")
+   S_(SECCLASS_MMU, MMU__MMUEXT_OP, "mmuext_op")
    S_(SECCLASS_SHADOW, SHADOW__DISABLE, "disable")
    S_(SECCLASS_SHADOW, SHADOW__ENABLE, "enable")
    S_(SECCLASS_SHADOW, SHADOW__LOGDIRTY, "logdirty")
diff --git a/xen/xsm/flask/include/av_permissions.h 
b/xen/xsm/flask/include/av_permissions.h
index b3831f6..18454fd 100644
--- a/xen/xsm/flask/include/av_permissions.h
+++ b/xen/xsm/flask/include/av_permissions.h
@@ -117,6 +117,7 @@
 #define MMU__MFNLIST                              0x00000400UL
 #define MMU__MEMORYMAP                            0x00000800UL
 #define MMU__REMOTE_REMAP                         0x00001000UL
+#define MMU__MMUEXT_OP                            0x00002000UL
 
 #define SHADOW__DISABLE                           0x00000001UL
 #define SHADOW__ENABLE                            0x00000002UL
-- 
1.7.11.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.