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>
---
xen/xsm/flask/hooks.c | 119 +++++++++++++++---------------
xen/xsm/flask/include/security.h | 8 ++
xen/xsm/flask/ss/services.c | 151 ++++++++++++++++++++++++++++++++------
3 files changed, 196 insertions(+), 82 deletions(-)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index e70feda..de7f249 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 8a64b0a..0dc21c8 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 6e72800..b880762 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;
}
--
1.7.7.3
