xsm/flask: fix resource list range checks

The FLASK security checks for resource ranges were not implemented
correctly - only the permissions on the endpoints of a range were
checked, instead of all items contained in the range. This would allow
certain resources (I/O ports, I/O memory) to be used by domains in
contravention to security policy.

This also corrects a bug where adding overlapping resource ranges did
not trigger an error.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Committed-by: Keir Fraser <keir@xen.org>

3 files changed, 196 insertions, 82 deletions

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index a17f02ab78..37b297e983 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -664,42 +664,47 @@ static int irq_has_perm(struct domain *d, uint8_t pirq, uint8_t access)
         return rc;
 }
 
-static int iomem_has_perm(struct domain *d, unsigned long mfn, uint8_t access)
-{
+struct iomem_has_perm_data {
+    struct domain_security_struct *ssec, *tsec;
     u32 perm;
-    u32 rsid;
-    int rc = -EPERM;
+};
 
-    struct domain_security_struct *ssec, *tsec;
+static int _iomem_has_perm(void *v, u32 sid, unsigned long start, unsigned long end)
+{
+    struct iomem_has_perm_data *data = v;
     struct avc_audit_data ad;
+    int rc = -EPERM;
 
-    rc = domain_has_perm(current->domain, d, SECCLASS_RESOURCE,
-                         resource_to_perm(access));
-    if ( rc )
-        return rc;
-
-    if ( access )
-        perm = RESOURCE__ADD_IOMEM;
-    else
-        perm = RESOURCE__REMOVE_IOMEM;
+    AVC_AUDIT_DATA_INIT(&ad, DEV);
+    ad.device = start;
 
-    ssec = current->domain->ssid;
-    tsec = d->ssid;
+    rc = avc_has_perm(data->ssec->sid, sid, SECCLASS_RESOURCE, data->perm, &ad);
 
-    rc = security_iomem_sid(mfn, &rsid);
     if ( rc )
         return rc;
 
-    AVC_AUDIT_DATA_INIT(&ad, DEV);
-    ad.device = mfn;
+    return avc_has_perm(data->tsec->sid, sid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
+}
 
-    rc = avc_has_perm(ssec->sid, rsid, SECCLASS_RESOURCE, perm, &ad);
+static int iomem_has_perm(struct domain *d, unsigned long start, unsigned long end, uint8_t access)
+{
+    struct iomem_has_perm_data data;
+    int rc;
 
+    rc = domain_has_perm(current->domain, d, SECCLASS_RESOURCE,
+                         resource_to_perm(access));
     if ( rc )
         return rc;
 
-    return avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, 
-                        RESOURCE__USE, &ad);
+    if ( access )
+        data.perm = RESOURCE__ADD_IOMEM;
+    else
+        data.perm = RESOURCE__REMOVE_IOMEM;
+
+    data.ssec = current->domain->ssid;
+    data.tsec = d->ssid;
+
+    return security_iterate_iomem_sids(start, end, _iomem_has_perm, &data);
 }
 
 static int flask_perfcontrol(void)
@@ -736,45 +741,49 @@ static int flask_shadow_control(struct domain *d, uint32_t op)
     return domain_has_perm(current->domain, d, SECCLASS_SHADOW, perm);
 }
 
-static int ioport_has_perm(struct domain *d, uint32_t ioport, uint8_t access)
-{
+struct ioport_has_perm_data {
+    struct domain_security_struct *ssec, *tsec;
     u32 perm;
-    u32 rsid;
-    int rc = -EPERM;
+};
 
+static int _ioport_has_perm(void *v, u32 sid, unsigned long start, unsigned long end)
+{
+    struct ioport_has_perm_data *data = v;
     struct avc_audit_data ad;
-    struct domain_security_struct *ssec, *tsec;
+    int rc;
 
-    rc = domain_has_perm(current->domain, d, SECCLASS_RESOURCE,
-                         resource_to_perm(access));
+    AVC_AUDIT_DATA_INIT(&ad, DEV);
+    ad.device = start;
+
+    rc = avc_has_perm(data->ssec->sid, sid, SECCLASS_RESOURCE, data->perm, &ad);
 
     if ( rc )
         return rc;
 
-    if ( access )
-        perm = RESOURCE__ADD_IOPORT;
-    else
-        perm = RESOURCE__REMOVE_IOPORT;
+    return avc_has_perm(data->tsec->sid, sid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
+}
 
-    ssec = current->domain->ssid;
-    tsec = d->ssid;
 
-    rc = security_ioport_sid(ioport, &rsid);
-    if ( rc )
-        return rc;
+static int ioport_has_perm(struct domain *d, uint32_t start, uint32_t end, uint8_t access)
+{
+    int rc;
+    struct ioport_has_perm_data data;
 
-    AVC_AUDIT_DATA_INIT(&ad, DEV);
-    ad.device = ioport;
+    rc = domain_has_perm(current->domain, d, SECCLASS_RESOURCE,
+                         resource_to_perm(access));
 
-    rc = avc_has_perm(ssec->sid, rsid, SECCLASS_RESOURCE, perm, &ad);
     if ( rc )
         return rc;
 
     if ( access )
-        return avc_has_perm(tsec->sid, rsid, SECCLASS_RESOURCE, 
-                            RESOURCE__USE, &ad);
+        data.perm = RESOURCE__ADD_IOPORT;
     else
-        return rc;
+        data.perm = RESOURCE__REMOVE_IOPORT;
+
+    data.ssec = current->domain->ssid;
+    data.tsec = d->ssid;
+
+    return security_iterate_ioport_sids(start, end, _ioport_has_perm, &data);
 }
 
 static int flask_getpageframeinfo(struct page_info *page)
@@ -1184,31 +1193,25 @@ static int io_has_perm(struct domain *d, char *name, unsigned long s,
 
     if ( strcmp(name, "I/O Memory") == 0 )
     {
-        rc = iomem_has_perm(d, s, access);
+        rc = iomem_has_perm(d, s, e, access);
         if ( rc )
             return rc;
-
-        if ( s != e )
-            rc = iomem_has_perm(d, e, access);
     }
     else if ( strcmp(name, "Interrupts") == 0 )
     {
-        rc = irq_has_perm(d, s, access);
-        if ( rc )
-            return rc;
-
-        if ( s != e )
-            rc = irq_has_perm(d, e, access);
+        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, access);
+        rc = ioport_has_perm(d, s, e, access);
         if ( rc )
             return rc;
-
-        if ( s != e )
-            rc = ioport_has_perm(d, e, access);
     }
 #endif
 
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index 8a64b0a3e5..0dc21c8c57 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -82,6 +82,14 @@ int security_device_sid(u32 device, u32 *out_sid);
 int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
                                                                     u16 tclass);
 
+typedef int (*security_iterate_fn)(void *data, u32 sid, unsigned long start,
+                                                        unsigned long end);
+int security_iterate_iomem_sids(unsigned long start, unsigned long end,
+                                security_iterate_fn fn, void *data);
+
+int security_iterate_ioport_sids(u32 start, u32 end,
+                                security_iterate_fn fn, void *data);
+
 int security_ocontext_add(char *ocontext, unsigned long low,
                            unsigned long high, u32 sid);
 
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index 6e728009e4..b880762313 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1594,6 +1594,53 @@ out:
     return rc;
 }
 
+int security_iterate_iomem_sids(unsigned long start, unsigned long end,
+                                security_iterate_fn fn, void *data)
+{
+    struct ocontext *c;
+    int rc = 0;
+
+    POLICY_RDLOCK;
+
+    c = policydb.ocontexts[OCON_IOMEM];
+    while (c && c->u.iomem.high_iomem < start)
+        c = c->next;
+
+    while (c && c->u.iomem.low_iomem <= end) {
+        if (!c->sid[0])
+        {
+            rc = sidtab_context_to_sid(&sidtab, &c->context[0], &c->sid[0]);
+            if ( rc )
+                goto out;
+        }
+        if (start < c->u.iomem.low_iomem) {
+            /* found a gap */
+            rc = fn(data, SECINITSID_IOMEM, start, c->u.iomem.low_iomem - 1);
+            if (rc)
+                goto out;
+            start = c->u.iomem.low_iomem;
+        }
+        if (end <= c->u.iomem.high_iomem) {
+            /* iteration ends in the middle of this range */
+            rc = fn(data, c->sid[0], start, end);
+            goto out;
+        }
+
+        rc = fn(data, c->sid[0], start, c->u.iomem.high_iomem);
+        if (rc)
+            goto out;
+        start = c->u.iomem.high_iomem + 1;
+
+        c = c->next;
+    }
+
+    rc = fn(data, SECINITSID_IOMEM, start, end);
+
+out:
+    POLICY_RDUNLOCK;
+    return rc;
+}
+
 /**
  * security_ioport_sid - Obtain the SID for an ioport.
  * @ioport: ioport
@@ -1635,6 +1682,53 @@ out:
     return rc;
 }
 
+int security_iterate_ioport_sids(u32 start, u32 end,
+                                security_iterate_fn fn, void *data)
+{
+    struct ocontext *c;
+    int rc = 0;
+
+    POLICY_RDLOCK;
+
+    c = policydb.ocontexts[OCON_IOPORT];
+    while (c && c->u.ioport.high_ioport < start)
+        c = c->next;
+
+    while (c && c->u.ioport.low_ioport <= end) {
+        if (!c->sid[0])
+        {
+            rc = sidtab_context_to_sid(&sidtab, &c->context[0], &c->sid[0]);
+            if ( rc )
+                goto out;
+        }
+        if (start < c->u.ioport.low_ioport) {
+            /* found a gap */
+            rc = fn(data, SECINITSID_IOPORT, start, c->u.ioport.low_ioport - 1);
+            if (rc)
+                goto out;
+            start = c->u.ioport.low_ioport;
+        }
+        if (end <= c->u.ioport.high_ioport) {
+            /* iteration ends in the middle of this range */
+            rc = fn(data, c->sid[0], start, end);
+            goto out;
+        }
+
+        rc = fn(data, c->sid[0], start, c->u.ioport.high_ioport);
+        if (rc)
+            goto out;
+        start = c->u.ioport.high_ioport + 1;
+
+        c = c->next;
+    }
+
+    rc = fn(data, SECINITSID_IOPORT, start, end);
+
+out:
+    POLICY_RDUNLOCK;
+    return rc;
+}
+
 /**
  * security_device_sid - Obtain the SID for a PCI device.
  * @ioport: device
@@ -1963,6 +2057,7 @@ int security_ocontext_add( char *ocontext, unsigned long low, unsigned long high
     int ret = 0;
     int ocon = 0;
     struct ocontext *c;
+    struct ocontext *prev;
     struct ocontext *add;
 
     if ( (ocon = determine_ocontext(ocontext)) < 0 )
@@ -2006,23 +2101,27 @@ int security_ocontext_add( char *ocontext, unsigned long low, unsigned long high
         add->u.ioport.low_ioport = low;
         add->u.ioport.high_ioport = high;
 
+        prev = NULL;
         c = policydb.ocontexts[OCON_IOPORT];
-        while ( c )
-        {
-            if ( c->u.ioport.low_ioport <= add->u.ioport.high_ioport &&
-                 add->u.ioport.low_ioport <= c->u.ioport.high_ioport )
-            {
-                printk("%s: IO Port overlap with entry 0x%x - 0x%x\n",
-                       __FUNCTION__, c->u.ioport.low_ioport,
-                       c->u.ioport.high_ioport);
-                ret = -EINVAL;
-                break;
-            }
+
+        while ( c && c->u.ioport.high_ioport < low ) {
+            prev = c;
             c = c->next;
         }
 
-        if ( ret == 0 )
+        if (c && c->u.ioport.low_ioport <= high)
         {
+            printk("%s: IO Port overlap with entry 0x%x - 0x%x\n",
+                   __FUNCTION__, c->u.ioport.low_ioport,
+                   c->u.ioport.high_ioport);
+            ret = -EINVAL;
+            break;
+        }
+
+        if (prev) {
+            add->next = prev->next;
+            prev->next = add;
+        } else {
             add->next = policydb.ocontexts[OCON_IOPORT];
             policydb.ocontexts[OCON_IOPORT] = add;
         }
@@ -2032,23 +2131,27 @@ int security_ocontext_add( char *ocontext, unsigned long low, unsigned long high
         add->u.iomem.low_iomem = low;
         add->u.iomem.high_iomem = high;
 
+        prev = NULL;
         c = policydb.ocontexts[OCON_IOMEM];
-        while ( c )
-        {
-            if ( c->u.iomem.low_iomem <= add->u.iomem.high_iomem &&
-                 add->u.iomem.low_iomem <= c->u.iomem.high_iomem )
-            {
-                printk("%s: IO Memory overlap with entry 0x%x - 0x%x\n",
-                       __FUNCTION__, c->u.iomem.low_iomem,
-                       c->u.iomem.high_iomem);
-                ret = -EINVAL;
-                break;
-            }
+
+        while ( c && c->u.iomem.high_iomem < low ) {
+            prev = c;
             c = c->next;
         }
 
-        if ( ret == 0 )
+        if (c && c->u.iomem.low_iomem <= high)
         {
+            printk("%s: IO Memory overlap with entry 0x%x - 0x%x\n",
+                   __FUNCTION__, c->u.iomem.low_iomem,
+                   c->u.iomem.high_iomem);
+            ret = -EINVAL;
+            break;
+        }
+
+        if (prev) {
+            add->next = prev->next;
+            prev->next = add;
+        } else {
             add->next = policydb.ocontexts[OCON_IOMEM];
             policydb.ocontexts[OCON_IOMEM] = add;
         }