References: bnc#813675 CVE-2013-1919 XSA-46

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)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

# Commit 934a5253d932b6f67fe40fc48975a2b0117e4cce
# Date 2013-05-21 11:32:34 +0200
# Author Jan Beulich <jbeulich@suse.com>
# Committer Jan Beulich <jbeulich@suse.com>
fix XSA-46 regression with xend/xm

The hypervisor side changes for XSA-46 require the tool stack to now
always map the guest pIRQ before granting access permission to the
underlying host IRQ (GSI). This in particular requires that pciif.py
no longer can skip this step (assuming qemu would do it) for HVM
guests.

This in turn exposes, however, an inconsistency between xend and qemu:
The former wants to always establish 1:1 mappings between pIRQ and host
IRQ (for non-MSI only of course), while the latter always wants to
allocate an arbitrary mapping. Since the whole tool stack obviously
should always agree on the mapping model, make libxc enforce the 1:1
mapping as the more natural one (as well as being the one that allows
for easier debugging, since there no need to find out the extra
mapping). Users of libxc that want to establish a particular (rather
than an allocated) mapping are still free to do so, as well as tool
stacks not based on libxc wanting to implement an allocation based
model (which is why it's not the hypervisor that's being changed to
enforce either model).

Since libxl, like xend, already uses a 1:1 model, it's unaffected by
the libxc change (and it being unaffected by the original hypervisor
side changes is - afaict - simply due to qemu getting spawned at a
later point in time compared to the xend event flow).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/tools/libxc/xc_physdev.c
+++ b/tools/libxc/xc_physdev.c
@@ -35,7 +35,7 @@ int xc_physdev_map_pirq(int xc_handle,
     map.domid = domid;
     map.type = MAP_PIRQ_TYPE_GSI;
     map.index = index;
-    map.pirq = *pirq;
+    map.pirq = *pirq < 0 ? index : *pirq;
 
     rc = do_physdev_op(xc_handle, PHYSDEVOP_map_pirq, &map, sizeof(map));
 
--- 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/tools/python/xen/xend/server/pciif.py
+++ b/tools/python/xen/xend/server/pciif.py
@@ -340,7 +340,7 @@ class PciController(DevController):
                 raise VmError(('pci: failed to configure I/O memory on device '+
                             '%s - errno=%d')%(dev.name,rc))
 
-        if not self.vm.info.is_hvm() and dev.irq:
+        if dev.irq > 0:
             rc = xc.physdev_map_pirq(domid = fe_domid,
                                    index = dev.irq,
                                    pirq  = dev.irq)
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -1091,7 +1091,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
@@ -907,9 +907,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 )
@@ -937,9 +941,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
@@ -176,6 +176,15 @@ int create_irq(void)
 out:
      spin_unlock_irqrestore(&vector_lock, flags);
 
+    if ( irq > 0 && 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;
 }
 
@@ -187,6 +196,16 @@ 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);
+    }
+
     spin_lock_irqsave(&desc->lock, flags);
     desc->status  |= IRQ_DISABLED;
     desc->status  &= ~IRQ_GUEST;
@@ -1590,7 +1609,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 )
@@ -1611,11 +1630,12 @@ int map_domain_pirq(
         return 0;
     }
 
-    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;
     }
 
@@ -1637,8 +1657,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;
+        }
         desc->handler = &pci_msi_type;
 
         if ( opt_irq_vector_map == OPT_IRQ_VECTOR_MAP_PERDEV
@@ -1667,6 +1693,10 @@ int map_domain_pirq(
     }
 
 done:
+    if ( ret && 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;
 }
 
@@ -1723,10 +1753,11 @@ int unmap_domain_pirq(struct domain *d, 
     if (msi_desc)
         msi_free_irq(msi_desc);
 
-    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);
 
     if ( desc->handler == &pci_msi_type )
         desc->handler = &no_irq_type;
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -82,7 +82,7 @@ static int physdev_map_pirq(struct physd
             if ( irq == -1 )
                 irq = create_irq();
 
-            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
@@ -851,9 +851,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( pirq >= d->nr_pirqs )
             ret = -EINVAL;
         else if ( op->u.irq_permission.allow_access )
-            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
@@ -305,7 +305,7 @@ static long evtchn_bind_pirq(evtchn_bind
     if ( (pirq < 0) || (pirq >= d->nr_pirqs) )
         return -EINVAL;
 
-    if ( !irq_access_permitted(d, pirq) )
+    if ( !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__ */
