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

[Xen-devel] [PATCH 5/8] xsm: Expand I/O resource hooks



The XSM hooks inside rangeset are not useful in capturing the PIRQ
mappings in HVM domains. They can also be called from softirq context
where current->domain is invalid, causing spurious AVC denials from
unrelated domains on such calls.

Within FLASK code, the rangeset hooks were already divided between IRQs,
I/O memory, and x86 IO ports; propagate this division back through the
XSM hooks and call the XSM functions directly when needed.

This removes XSM checks for the initial rangeset population for dom0 and
the removal checks on domain destruction; denying either of these
actions does not make sense.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
 xen/arch/x86/domctl.c  |   24 +++++++++++++--
 xen/arch/x86/irq.c     |    9 ++++++
 xen/arch/x86/physdev.c |    4 ++
 xen/common/domctl.c    |   10 +++++-
 xen/common/rangeset.c  |    8 -----
 xen/include/xsm/xsm.h  |   22 ++++++++------
 xen/xsm/dummy.c        |   14 ++++++---
 xen/xsm/flask/hooks.c  |   73 ++++++++++++------------------------------------
 8 files changed, 81 insertions(+), 83 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 5ed211f..a676f79 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -76,6 +76,7 @@ long arch_do_domctl(
         struct domain *d;
         unsigned int fp = domctl->u.ioport_permission.first_port;
         unsigned int np = domctl->u.ioport_permission.nr_ports;
+        int allow = domctl->u.ioport_permission.allow_access;
 
         ret = -EINVAL;
         if ( (fp + np) > 65536 )
@@ -87,7 +88,9 @@ long arch_do_domctl(
 
         if ( np == 0 )
             ret = 0;
-        else if ( domctl->u.ioport_permission.allow_access )
+        else if ( xsm_ioport_permission(d, fp, fp + np - 1, allow) )
+            ret = -EPERM;
+        else if ( allow )
             ret = ioports_permit_access(d, fp, fp + np - 1);
         else
             ret = ioports_deny_access(d, fp, fp + np - 1);
@@ -822,6 +825,7 @@ long arch_do_domctl(
         unsigned long gfn = domctl->u.memory_mapping.first_gfn;
         unsigned long mfn = domctl->u.memory_mapping.first_mfn;
         unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
+        int add = domctl->u.memory_mapping.add_mapping;
         int i;
 
         ret = -EINVAL;
@@ -837,8 +841,13 @@ long arch_do_domctl(
         if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
             break;
 
-        ret=0;
-        if ( domctl->u.memory_mapping.add_mapping )
+        ret = xsm_iomem_permission(d, mfn, mfn + nr_mfns - 1, add);
+        if ( ret ) {
+            rcu_unlock_domain(d);
+            break;
+        }
+
+        if ( add )
         {
             gdprintk(XENLOG_INFO,
                 "memory_map:add: gfn=%lx mfn=%lx nr_mfns=%lx\n",
@@ -871,6 +880,7 @@ long arch_do_domctl(
         unsigned int fgp = domctl->u.ioport_mapping.first_gport;
         unsigned int fmp = domctl->u.ioport_mapping.first_mport;
         unsigned int np = domctl->u.ioport_mapping.nr_ports;
+        unsigned int add = domctl->u.ioport_mapping.add_mapping;
         struct g2m_ioport *g2m_ioport;
         int found = 0;
 
@@ -893,8 +903,14 @@ long arch_do_domctl(
         if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
             break;
 
+        ret = xsm_ioport_permission(d, fmp, fmp + np - 1, add);
+        if ( ret ) {
+            rcu_unlock_domain(d);
+            break;
+        }
+
         hd = domain_hvm_iommu(d);
-        if ( domctl->u.ioport_mapping.add_mapping )
+        if ( add )
         {
             gdprintk(XENLOG_INFO,
                 "ioport_map:add f_gport=%x f_mport=%x np=%x\n",
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 9149096..b1c5d42 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -18,6 +18,7 @@
 #include <xen/iocap.h>
 #include <xen/iommu.h>
 #include <xen/trace.h>
+#include <xsm/xsm.h>
 #include <asm/msi.h>
 #include <asm/current.h>
 #include <asm/flushtlb.h>
@@ -1817,6 +1818,14 @@ int map_domain_pirq(
         return 0;
     }
 
+    ret = xsm_irq_permission(d, irq, 1);
+    if ( ret )
+    {
+        dprintk(XENLOG_G_ERR, "dom%d: could not permit access to irq %d 
mapping to pirq %d\n",
+                d->domain_id, irq, pirq);
+        return ret;
+    }
+
     ret = irq_permit_access(d, pirq);
     if ( ret )
     {
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 5c7ab68..5a4acae 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -229,6 +229,10 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
     if ( !IS_PRIV_FOR(current->domain, d) )
         goto free_domain;
 
+    ret = xsm_irq_permission(d, pirq, 0);
+    if ( ret )
+        goto free_domain;
+
     spin_lock(&pcidevs_lock);
     spin_lock(&d->event_lock);
     ret = unmap_domain_pirq(d, pirq);
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 6705a57..06594a0 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -858,6 +858,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
     {
         struct domain *d;
         unsigned int pirq = op->u.irq_permission.pirq;
+        int allow = op->u.irq_permission.allow_access;
 
         ret = -ESRCH;
         d = rcu_lock_domain_by_id(op->domain);
@@ -866,7 +867,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
 
         if ( pirq >= d->nr_pirqs )
             ret = -EINVAL;
-        else if ( op->u.irq_permission.allow_access )
+        else if ( xsm_irq_permission(d, pirq, allow) )
+            ret = -EPERM;
+        else if ( allow )
             ret = irq_permit_access(d, pirq);
         else
             ret = irq_deny_access(d, pirq);
@@ -880,6 +883,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
         struct domain *d;
         unsigned long mfn = op->u.iomem_permission.first_mfn;
         unsigned long nr_mfns = op->u.iomem_permission.nr_mfns;
+        int allow = op->u.iomem_permission.allow_access;
 
         ret = -EINVAL;
         if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */
@@ -890,7 +894,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
         if ( d == NULL )
             break;
 
-        if ( op->u.iomem_permission.allow_access )
+        if ( xsm_iomem_permission(d, mfn, mfn + nr_mfns - 1, allow) )
+            ret = -EPERM;
+        else if ( allow )
             ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
         else
             ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index bb9523f..f09c0c4 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -97,10 +97,6 @@ int rangeset_add_range(
     struct range *x, *y;
     int rc = 0;
 
-    rc = xsm_add_range(r->domain, r->name, s, e);
-    if ( rc )
-        return rc;
-
     ASSERT(s <= e);
 
     spin_lock(&r->lock);
@@ -169,10 +165,6 @@ int rangeset_remove_range(
     struct range *x, *y, *t;
     int rc = 0;
 
-    rc = xsm_remove_range(r->domain, r->name, s, e);
-    if ( rc )
-        return rc;
-
     ASSERT(s <= e);
 
     spin_lock(&r->lock);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index da1f5d0..45fee21 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -106,8 +106,8 @@ struct xsm_operations {
 
     int (*kexec) (void);
     int (*schedop_shutdown) (struct domain *d1, struct domain *d2);
-    int (*add_range) (struct domain *d, char *name, unsigned long s, unsigned 
long e);
-    int (*remove_range) (struct domain *d, char *name, unsigned long s, 
unsigned long e);
+    int (*irq_permission) (struct domain *d, int pirq, uint8_t allow);
+    int (*iomem_permission) (struct domain *d, uint64_t s, uint64_t e, uint8_t 
allow);
 
     int (*test_assign_device) (uint32_t machine_bdf);
     int (*assign_device) (struct domain *d, uint32_t machine_bdf);
@@ -152,6 +152,7 @@ struct xsm_operations {
     int (*pin_mem_cacheattr) (struct domain *d);
     int (*ext_vcpucontext) (struct domain *d, uint32_t cmd);
     int (*vcpuextstate) (struct domain *d, uint32_t cmd);
+    int (*ioport_permission) (struct domain *d, uint32_t s, uint32_t e, 
uint8_t allow);
 #endif
 };
 
@@ -415,16 +416,14 @@ static inline int xsm_schedop_shutdown (struct domain 
*d1, struct domain *d2)
     return xsm_call(schedop_shutdown(d1, d2));
 }
 
-static inline int xsm_add_range (struct domain *d, char *name, unsigned long s,
-                                                                        
unsigned long e)
+static inline int xsm_irq_permission (struct domain *d, int pirq, uint8_t 
allow)
 {
-    return xsm_call(add_range(d, name, s, e));
+    return xsm_call(irq_permission(d, pirq, allow));
 }
- 
-static inline int xsm_remove_range (struct domain *d, char *name, unsigned 
long s,
-                                                                        
unsigned long e)
+
+static inline int xsm_iomem_permission (struct domain *d, uint64_t s, uint64_t 
e, uint8_t allow)
 {
-    return xsm_call(remove_range(d, name, s, e));
+    return xsm_call(iomem_permission(d, s, e, allow));
 }
 
 static inline int xsm_test_assign_device(uint32_t machine_bdf)
@@ -639,6 +638,11 @@ static inline int xsm_vcpuextstate(struct domain *d, 
uint32_t cmd)
 {
     return xsm_call(vcpuextstate(d, cmd));
 }
+
+static inline int xsm_ioport_permission (struct domain *d, uint32_t s, 
uint32_t e, uint8_t allow)
+{
+    return xsm_call(ioport_permission(d, s, e, allow));
+}
 #endif /* CONFIG_X86 */
 
 extern struct xsm_operations dummy_xsm_ops;
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index ef461e6..a629396 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -273,13 +273,12 @@ static long dummy___do_xsm_op(XEN_GUEST_HANDLE(xsm_op_t) 
op)
     return -ENOSYS;
 }
 
-static int dummy_add_range (struct domain *d, char *name, unsigned long s, 
unsigned long e)
+static int dummy_irq_permission (struct domain *d, int pirq, uint8_t allow)
 {
     return 0;
 }
 
-static int dummy_remove_range (struct domain *d, char *name, unsigned long s, 
-                                                                        
unsigned long e)
+static int dummy_iomem_permission (struct domain *d, uint64_t s, uint64_t e, 
uint8_t allow)
 {
     return 0;
 }
@@ -462,6 +461,10 @@ static int dummy_vcpuextstate (struct domain *d, uint32_t 
cmd)
     return 0;
 }
 
+static int dummy_ioport_permission (struct domain *d, uint32_t s, uint32_t e, 
uint8_t allow)
+{
+    return 0;
+}
 #endif
 
 struct xsm_operations dummy_xsm_ops;
@@ -536,8 +539,8 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, kexec);
     set_to_dummy_if_null(ops, schedop_shutdown);
 
-    set_to_dummy_if_null(ops, add_range);
-    set_to_dummy_if_null(ops, remove_range);
+    set_to_dummy_if_null(ops, irq_permission);
+    set_to_dummy_if_null(ops, iomem_permission);
 
     set_to_dummy_if_null(ops, __do_xsm_op);
 
@@ -577,5 +580,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, pin_mem_cacheattr);
     set_to_dummy_if_null(ops, ext_vcpucontext);
     set_to_dummy_if_null(ops, vcpuextstate);
+    set_to_dummy_if_null(ops, ioport_permission);
 #endif
 }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 80c1f70..1bea498 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -643,7 +643,7 @@ static inline u32 resource_to_perm(uint8_t access)
         return RESOURCE__REMOVE;
 }
 
-static int irq_has_perm(struct domain *d, uint8_t pirq, uint8_t access)
+static int flask_irq_permission (struct domain *d, int pirq, uint8_t access)
 {
     u32 perm;
     u32 rsid;
@@ -678,10 +678,9 @@ static int irq_has_perm(struct domain *d, uint8_t pirq, 
uint8_t access)
         return rc;
 
     if ( access )
-        return avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, 
+        rc = avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, 
                             RESOURCE__USE, &ad);
-    else
-        return rc;
+    return rc;
 }
 
 struct iomem_has_perm_data {
@@ -706,7 +705,7 @@ static int _iomem_has_perm(void *v, u32 sid, unsigned long 
start, unsigned long
     return avc_has_perm(data->tsec->sid, sid, SECCLASS_RESOURCE, 
RESOURCE__USE, &ad);
 }
 
-static int iomem_has_perm(struct domain *d, unsigned long start, unsigned long 
end, uint8_t access)
+static int flask_iomem_permission(struct domain *d, uint64_t start, uint64_t 
end, uint8_t access)
 {
     struct iomem_has_perm_data data;
     int rc;
@@ -784,7 +783,7 @@ static int _ioport_has_perm(void *v, u32 sid, unsigned long 
start, unsigned long
 }
 
 
-static int ioport_has_perm(struct domain *d, uint32_t start, uint32_t end, 
uint8_t access)
+static int flask_ioport_permission(struct domain *d, uint32_t start, uint32_t 
end, uint8_t access)
 {
     int rc;
     struct ioport_has_perm_data data;
@@ -1142,23 +1141,30 @@ static int flask_bind_pt_irq (struct domain *d, struct 
xen_domctl_bind_pt_irq *b
 {
     u32 rsid;
     int rc = -EPERM;
+    int irq;
     struct domain_security_struct *ssec, *tsec;
+    struct avc_audit_data ad;
 
     rc = domain_has_perm(current->domain, d, SECCLASS_RESOURCE, RESOURCE__ADD);
     if ( rc )
         return rc;
 
-    rc = security_pirq_sid(bind->machine_irq, &rsid);
+    irq = domain_pirq_to_irq(d, bind->machine_irq);
+
+    rc = security_pirq_sid(irq, &rsid);
     if ( rc )
         return rc;
 
+    AVC_AUDIT_DATA_INIT(&ad, DEV);
+    ad.device = (unsigned long)irq;
+
     ssec = current->domain->ssid;
-    rc = avc_has_perm(ssec->sid, rsid, SECCLASS_HVM, HVM__BIND_IRQ, NULL);
+    rc = avc_has_perm(ssec->sid, rsid, SECCLASS_HVM, HVM__BIND_IRQ, &ad);
     if ( rc )
         return rc;
 
     tsec = d->ssid;
-    return avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, 
NULL);
+    return avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, 
&ad);
 }
 
 static int flask_pin_mem_cacheattr (struct domain *d)
@@ -1205,50 +1211,6 @@ static int flask_vcpuextstate (struct domain *d, 
uint32_t cmd)
 }
 #endif
 
-static int io_has_perm(struct domain *d, char *name, unsigned long s, 
-                       unsigned long e, u32 access)
-{
-    int rc = -EPERM;
-
-    if ( strcmp(name, "I/O Memory") == 0 )
-    {
-        rc = iomem_has_perm(d, s, e, access);
-        if ( rc )
-            return rc;
-    }
-    else if ( strcmp(name, "Interrupts") == 0 )
-    {
-        while (s <= e) {
-            rc = irq_has_perm(d, s, access);
-            if ( rc )
-                return rc;
-            s++;
-        }
-    }
-#ifdef CONFIG_X86
-    else if ( strcmp(name, "I/O Ports") == 0 )
-    {
-        rc = ioport_has_perm(d, s, e, access);
-        if ( rc )
-            return rc;
-    }
-#endif
-
-    return rc;    
-}
-
-static int flask_add_range(struct domain *d, char *name, unsigned long s,
-                           unsigned long e)
-{
-    return io_has_perm(d, name, s, e, 1);
-}
-
-static int flask_remove_range(struct domain *d, char *name, unsigned long s,
-                              unsigned long e)
-{
-    return io_has_perm(d, name, s, e, 0);
-}
-
 long do_flask_op(XEN_GUEST_HANDLE(xsm_op_t) u_flask_op);
 
 static struct xsm_operations flask_ops = {
@@ -1308,8 +1270,8 @@ static struct xsm_operations flask_ops = {
     .kexec = flask_kexec,
     .schedop_shutdown = flask_schedop_shutdown,
 
-    .add_range = flask_add_range,
-    .remove_range = flask_remove_range,
+    .irq_permission = flask_irq_permission,
+    .iomem_permission = flask_iomem_permission,
 
     .__do_xsm_op = do_flask_op,
 
@@ -1348,6 +1310,7 @@ static struct xsm_operations flask_ops = {
     .pin_mem_cacheattr = flask_pin_mem_cacheattr,
     .ext_vcpucontext = flask_ext_vcpucontext,
     .vcpuextstate = flask_vcpuextstate,
+    .ioport_permission = flask_ioport_permission,
 #endif
 };
 
-- 
1.7.7.3


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

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