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>

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 19a56c0c778d3faa423cec2cd0a264b03dedbc2f..cb9c8227858a594ae57f84b809b70c34c4551853 100644 (file)
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -966,14 +966,16 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
     }
 
     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;
         }
     }

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 c8f435db32b5694ff9f70276d35552571e0efb69..7ae084f21b95bd9b49a6cf424ea672c87dc8f21a 100644 (file)
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -1080,7 +1080,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 a196e2a870d29a94e9c4b9d96d4e535aaecb5585..8fb4fa98a597d5d0233dfb0db3a2b6aac464b4ce 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -578,9 +578,13 @@ long arch_do_domctl(
             break;
 
         ret = -EPERM;
-        if ( !IS_PRIV(current->domain) &&
-             !irq_access_permitted(current->domain, bind->machine_irq) )
-            break;
+        if ( !IS_PRIV(current->domain) )
+        {
+            int irq = domain_pirq_to_irq(d, bind->machine_irq);
+
+            if ( irq <= 0 || !irq_access_permitted(current->domain, irq) )
+                break;
+        }
 
         ret = -ESRCH;
         if ( iommu_enabled )
@@ -602,9 +606,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) )
-            break;
+        if ( !IS_PRIV(current->domain) )
+        {
+            int irq = domain_pirq_to_irq(d, bind->machine_irq);
+
+            if ( irq <= 0 || !irq_access_permitted(current->domain, irq) )
+                break;
+        }
 
         ret = xsm_unbind_pt_irq(XSM_HOOK, d, bind);
         if ( ret )

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index fa6b9a2e311396432256b40f4380de58e0785c87..bbf413089dfe0218b0e4a8ab2affaddffdb232ab 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -185,6 +185,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;
 }
@@ -281,6 +289,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);
 }
@@ -1873,7 +1892,7 @@ int map_domain_pirq(
     ASSERT(spin_is_locked(&d->event_lock));
 
     if ( !IS_PRIV(current->domain) &&
-         !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 )
@@ -1902,17 +1921,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);
 
@@ -1936,8 +1956,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;
+        }
 
         ret = setup_msi_irq(desc, msi_desc);
         if ( ret )
@@ -1972,7 +1998,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;
 }
 
@@ -2043,10 +2076,11 @@ int unmap_domain_pirq(struct domain *d, int pirq)
     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;

diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 876ac9d623d6ffc2e176a8358a476f10263cf43e..eb8a40706a0fcd7f311a6f787a9a3f25296bdbdb 100644 (file)
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -144,7 +144,7 @@ int physdev_map_pirq(domid_t domid, int type, int *index, int *pirq_p,
         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);

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 6bd8efdf063648fa71857d07707db635502baa42..73b12c8a4df3b1816c6ead29ea9b1d95d453c549 100644 (file)
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -25,6 +25,7 @@
 #include <xen/paging.h>
 #include <xen/hypercall.h>
 #include <asm/current.h>
+#include <asm/irq.h>
 #include <asm/page.h>
 #include <public/domctl.h>
 #include <xsm/xsm.h>
@@ -777,9 +778,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         else if ( xsm_irq_permission(XSM_HOOK, 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);
     }
     break;
 

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 0a6684c6d57568b08e2fef30d38157ba905a0002..64c976b20a565fbc2d2f2d57d7f215eeaed8fe18 100644 (file)
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -369,7 +369,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__ */