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 <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
master commit: 545607eb3cfeb2abf5742d1bb869734f317fcfe5
master date: 2013-04-18 16:11:23 +0200

diff --git a/tools/python/xen/xend/server/irqif.py b/tools/python/xen/xend/server/irqif.py
index ae0b1ff4b6ef48626a484d91be6a9b9af12099d9..723f34652a56427142e2cb7abd7a68b4b3975cf9 100644 (file)
--- 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, {})
 

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 82850484c0f269d27260ff8abb6e32baac90c356..93215d239ce85cbda2a7f299cdd57dc8242c7751 100644 (file)
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -1201,7 +1201,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.

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index b285003b8fdadee4d3b5980dc6ef29f292e46d09..90901396fda3029cf4cea97ef9351cbfb644925e 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -908,9 +908,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 )
@@ -938,9 +942,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 )
         {

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 3f6b6a72d5846a0c4de1cb5ed669ad6dc6aabd35..7e2c212dd537003340f38ddbd0462b7f62331fae 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -174,6 +174,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;
 }
 
@@ -258,6 +267,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);
 }
@@ -1604,7 +1624,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 )
@@ -1625,11 +1645,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;
     }
 
@@ -1651,8 +1672,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
              && !desc->chip_data->used_vectors )
@@ -1680,6 +1707,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;
 }
 
@@ -1736,10 +1767,11 @@ int unmap_domain_pirq(struct domain *d, int pirq)
     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;

diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 8feb84aa67a76030b7f59236eb36d8678860926f..bf1ff16144221818a5d1b903ad53186401b50746 100644 (file)
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -147,7 +147,7 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
         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);

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 981cb1a6dfaa4f9da03805e0eda7a879c053f5c4..d9a1f532d065a3821ae860c86c9b418aaade459d 100644 (file)
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -854,9 +854,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 )
-            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);
     }

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 5c7bdb6e62bc013eec4e032ddcb52c67e53d1999..e4a779d9bac748f5c8fac50badbb8743cf00da97 100644 (file)
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -332,7 +332,7 @@ static long evtchn_bind_pirq(evtchn_bind_pirq_t *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);

diff --git a/xen/include/xen/iocap.h b/xen/include/xen/iocap.h
index 63bb49f140d8d129883cf5b4a8c3735cc31c7b2c..b755ecbdb374ffcd22d859fd4ca08d8e21353145 100644 (file)
--- 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__ */