[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] 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@xxxxxxxxxxxxx> --- 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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |