x86: fix various issues with handling guest IRQs - properly revoke IRQ access in map_domain_pirq() error path - don't permit replacing an in use IRQ - don't accept inputs in the GSI range for MAP_PIRQ_TYPE_MSI - track IRQ access permission in host IRQ terms, not guest IRQ ones (and with that, also disallow Dom0 access to IRQ0) This is CVE-2013-1919 / XSA-46. Signed-off-by: Jan Beulich Acked-by: Stefano Stabellini --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -968,14 +968,16 @@ static void domcreate_launch_dm(libxl__e } for (i = 0; i < d_config->b_info.num_irqs; i++) { - uint32_t irq = d_config->b_info.irqs[i]; + int irq = d_config->b_info.irqs[i]; - LOG(DEBUG, "dom%d irq %"PRIx32, domid, irq); + LOG(DEBUG, "dom%d irq %d", domid, irq); - ret = xc_domain_irq_permission(CTX->xch, domid, irq, 1); + ret = irq >= 0 ? xc_physdev_map_pirq(CTX->xch, domid, irq, &irq) + : -EOVERFLOW; + if (!ret) + ret = xc_domain_irq_permission(CTX->xch, domid, irq, 1); if ( ret<0 ){ - LOGE(ERROR, - "failed give dom%d access to irq %"PRId32, domid, irq); + LOGE(ERROR, "failed give dom%d access to irq %d", domid, irq); ret = ERROR_FAIL; } } --- a/tools/python/xen/xend/server/irqif.py +++ b/tools/python/xen/xend/server/irqif.py @@ -73,6 +73,12 @@ class IRQController(DevController): pirq = get_param('irq') + rc = xc.physdev_map_pirq(domid = self.getDomid(), + index = pirq, + pirq = pirq) + if rc < 0: + raise VmError('irq: Failed to map irq %x' % (pirq)) + rc = xc.domain_irq_permission(domid = self.getDomid(), pirq = pirq, allow_access = True) @@ -81,12 +87,6 @@ class IRQController(DevController): #todo non-fatal raise VmError( 'irq: Failed to configure irq: %d' % (pirq)) - rc = xc.physdev_map_pirq(domid = self.getDomid(), - index = pirq, - pirq = pirq) - if rc < 0: - raise VmError( - 'irq: Failed to map irq %x' % (pirq)) back = dict([(k, config[k]) for k in self.valid_cfg if k in config]) return (self.allocateDeviceID(), back, {}) --- a/xen/arch/x86/domain_build.c +++ b/xen/arch/x86/domain_build.c @@ -1219,7 +1219,7 @@ int __init construct_dom0( /* DOM0 is permitted full I/O capabilities. */ rc |= ioports_permit_access(dom0, 0, 0xFFFF); rc |= iomem_permit_access(dom0, 0UL, ~0UL); - rc |= irqs_permit_access(dom0, 0, d->nr_pirqs - 1); + rc |= irqs_permit_access(dom0, 1, nr_irqs_gsi - 1); /* * Modify I/O port access permissions. --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -772,9 +772,13 @@ long arch_do_domctl( goto bind_out; ret = -EPERM; - if ( !IS_PRIV(current->domain) && - !irq_access_permitted(current->domain, bind->machine_irq) ) - goto bind_out; + if ( !IS_PRIV(current->domain) ) + { + int irq = domain_pirq_to_irq(d, bind->machine_irq); + + if ( irq <= 0 || !irq_access_permitted(current->domain, irq) ) + goto bind_out; + } ret = -ESRCH; if ( iommu_enabled ) @@ -803,9 +807,13 @@ long arch_do_domctl( bind = &(domctl->u.bind_pt_irq); ret = -EPERM; - if ( !IS_PRIV(current->domain) && - !irq_access_permitted(current->domain, bind->machine_irq) ) - goto unbind_out; + if ( !IS_PRIV(current->domain) ) + { + int irq = domain_pirq_to_irq(d, bind->machine_irq); + + if ( irq <= 0 || !irq_access_permitted(current->domain, irq) ) + goto unbind_out; + } if ( iommu_enabled ) { --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -184,6 +184,14 @@ int create_irq(int node) desc->arch.used = IRQ_UNUSED; irq = ret; } + else if ( dom0 ) + { + ret = irq_permit_access(dom0, irq); + if ( ret ) + printk(XENLOG_G_ERR + "Could not grant Dom0 access to IRQ%d (error %d)\n", + irq, ret); + } return irq; } @@ -280,6 +288,17 @@ void clear_irq_vector(int irq) void destroy_irq(unsigned int irq) { BUG_ON(!MSI_IRQ(irq)); + + if ( dom0 ) + { + int err = irq_deny_access(dom0, irq); + + if ( err ) + printk(XENLOG_G_ERR + "Could not revoke Dom0 access to IRQ%u (error %d)\n", + irq, err); + } + dynamic_irq_cleanup(irq); clear_irq_vector(irq); } @@ -1858,7 +1877,7 @@ int map_domain_pirq( if ( !IS_PRIV(current->domain) && !(IS_PRIV_FOR(current->domain, d) && - irq_access_permitted(current->domain, pirq))) + irq_access_permitted(current->domain, irq))) return -EPERM; if ( pirq < 0 || pirq >= d->nr_pirqs || irq < 0 || irq >= nr_irqs ) @@ -1887,17 +1906,18 @@ int map_domain_pirq( return ret; } - ret = irq_permit_access(d, pirq); + ret = irq_permit_access(d, irq); if ( ret ) { - dprintk(XENLOG_G_ERR, "dom%d: could not permit access to irq %d\n", - d->domain_id, pirq); + printk(XENLOG_G_ERR + "dom%d: could not permit access to IRQ%d (pirq %d)\n", + d->domain_id, irq, pirq); return ret; } ret = prepare_domain_irq_pirq(d, irq, pirq, &info); if ( ret ) - return ret; + goto revoke; desc = irq_to_desc(irq); @@ -1921,8 +1941,14 @@ int map_domain_pirq( spin_lock_irqsave(&desc->lock, flags); if ( desc->handler != &no_irq_type ) + { + spin_unlock_irqrestore(&desc->lock, flags); dprintk(XENLOG_G_ERR, "dom%d: irq %d in use\n", d->domain_id, irq); + pci_disable_msi(msi_desc); + ret = -EBUSY; + goto done; + } setup_msi_handler(desc, msi_desc); if ( opt_irq_vector_map == OPT_IRQ_VECTOR_MAP_PERDEV @@ -1951,7 +1977,14 @@ int map_domain_pirq( done: if ( ret ) + { cleanup_domain_irq_pirq(d, irq, info); + revoke: + if ( irq_deny_access(d, irq) ) + printk(XENLOG_G_ERR + "dom%d: could not revoke access to IRQ%d (pirq %d)\n", + d->domain_id, irq, pirq); + } return ret; } @@ -2017,10 +2050,11 @@ int unmap_domain_pirq(struct domain *d, if ( !forced_unbind ) cleanup_domain_irq_pirq(d, irq, info); - ret = irq_deny_access(d, pirq); + ret = irq_deny_access(d, irq); if ( ret ) - dprintk(XENLOG_G_ERR, "dom%d: could not deny access to irq %d\n", - d->domain_id, pirq); + printk(XENLOG_G_ERR + "dom%d: could not deny access to IRQ%d (pirq %d)\n", + d->domain_id, irq, pirq); done: return ret; --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -147,7 +147,7 @@ int physdev_map_pirq(domid_t domid, int if ( irq == -1 ) irq = create_irq(NUMA_NO_NODE); - if ( irq < 0 || irq >= nr_irqs ) + if ( irq < nr_irqs_gsi || irq >= nr_irqs ) { dprintk(XENLOG_G_ERR, "dom%d: can't create irq for msi!\n", d->domain_id); --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -897,9 +898,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc else if ( xsm_irq_permission(d, pirq, allow) ) ret = -EPERM; else if ( allow ) - ret = irq_permit_access(d, pirq); + ret = pirq_permit_access(d, pirq); else - ret = irq_deny_access(d, pirq); + ret = pirq_deny_access(d, pirq); rcu_unlock_domain(d); } --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -369,7 +369,7 @@ static long evtchn_bind_pirq(evtchn_bind if ( (pirq < 0) || (pirq >= d->nr_pirqs) ) return -EINVAL; - if ( !is_hvm_domain(d) && !irq_access_permitted(d, pirq) ) + if ( !is_hvm_domain(d) && !pirq_access_permitted(d, pirq) ) return -EPERM; spin_lock(&d->event_lock); --- a/xen/include/xen/iocap.h +++ b/xen/include/xen/iocap.h @@ -28,4 +28,22 @@ #define irq_access_permitted(d, i) \ rangeset_contains_singleton((d)->irq_caps, i) +#define pirq_permit_access(d, i) ({ \ + struct domain *d__ = (d); \ + int i__ = domain_pirq_to_irq(d__, i); \ + i__ > 0 ? rangeset_add_singleton(d__->irq_caps, i__)\ + : -EINVAL; \ +}) +#define pirq_deny_access(d, i) ({ \ + struct domain *d__ = (d); \ + int i__ = domain_pirq_to_irq(d__, i); \ + i__ > 0 ? rangeset_remove_singleton(d__->irq_caps, i__)\ + : -EINVAL; \ +}) +#define pirq_access_permitted(d, i) ({ \ + struct domain *d__ = (d); \ + rangeset_contains_singleton(d__->irq_caps, \ + domain_pirq_to_irq(d__, i));\ +}) + #endif /* __XEN_IOCAP_H__ */