[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |