domain: locking for irq_caps accesses

In order to be able to pull at least the XEN_DOMCTL_{,un}bind_pt_irq
handling out of the domctl-locked region, a separate (per-domain) lock is
needed to synchronize in particular with XEN_DOMCTL_{irq,gsi}_permission.

Locking is added only as far as domctl-s are affected. Uses presently
outside of the domctl lock may want dealing with subsequently (perhaps
limited to non-__init code).

This is part of XSA-492.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <julien@xen.org>

xen/arch/arm/domctl.c

@@ -76,6 +76,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
     case XEN_DOMCTL_bind_pt_irq:
     {
         int rc;
+        struct domain *currd = current->domain;
         struct xen_domctl_bind_pt_irq *bind = &domctl->u.bind_pt_irq;
         uint32_t irq = bind->u.spi.spi;
         uint32_t virq = bind->machine_irq;
@@ -107,21 +108,26 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
         if ( rc )
             return rc;
 
-        if ( !irq_access_permitted(current->domain, irq) )
-            return -EPERM;
+        read_lock(&currd->caps_lock);
 
-        if ( !vgic_reserve_virq(d, virq) )
-            return -EBUSY;
-
-        rc = route_irq_to_guest(d, virq, irq, "routed IRQ");
-        if ( rc )
-            vgic_free_virq(d, virq);
+        if ( !irq_access_permitted(currd, irq) )
+            rc = -EPERM;
+        else if ( !vgic_reserve_virq(d, virq) )
+            rc = -EBUSY;
+        else
+        {
+            rc = route_irq_to_guest(d, virq, irq, "routed IRQ");
+            if ( rc )
+                vgic_free_virq(d, virq);
+        }
 
+        read_unlock(&currd->caps_lock);
         return rc;
     }
     case XEN_DOMCTL_unbind_pt_irq:
     {
         int rc;
+        struct domain *currd = current->domain;
         struct xen_domctl_bind_pt_irq *bind = &domctl->u.bind_pt_irq;
         uint32_t irq = bind->u.spi.spi;
         uint32_t virq = bind->machine_irq;
@@ -138,16 +144,15 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
         if ( rc )
             return rc;
 
-        if ( !irq_access_permitted(current->domain, irq) )
-            return -EPERM;
+        read_lock(&currd->caps_lock);
 
-        rc = release_guest_irq(d, virq);
-        if ( rc )
-            return rc;
+        if ( !irq_access_permitted(currd, irq) )
+            rc = -EPERM;
+        else if ( !(rc = release_guest_irq(d, virq)) )
+            vgic_free_virq(d, virq);
 
-        vgic_free_virq(d, virq);
-
-        return 0;
+        read_unlock(&currd->caps_lock);
+        return rc;
     }
 
     case XEN_DOMCTL_vuart_op:

xen/arch/x86/domctl.c

@@ -271,16 +271,17 @@ long arch_do_domctl(
             break;
         }
 
-        ret = -EPERM;
+        iocaps_double_lock(d, true);
+
         if ( !irq_access_permitted(currd, irq) ||
              xsm_irq_permission(XSM_HOOK, d, irq, flags) )
-            break;
-
-        if ( flags )
+            ret = -EPERM;
+        else if ( flags )
             ret = irq_permit_access(d, irq);
         else
             ret = irq_deny_access(d, irq);
 
+        iocaps_double_unlock(d, true);
         break;
     }
 
@@ -583,20 +584,27 @@ long arch_do_domctl(
             break;
 
         irq = domain_pirq_to_irq(d, bind->machine_irq);
-        ret = -EPERM;
-        if ( irq <= 0 || !irq_access_permitted(currd, irq) )
-            break;
+        if ( irq <= 0 )
+            ret = -EPERM;
 
-        ret = -ESRCH;
-        if ( is_iommu_enabled(d) )
+        read_lock(&currd->caps_lock);
+
+        if ( !irq_access_permitted(currd, irq) )
+            ret = -EPERM;
+        else if ( is_iommu_enabled(d) )
         {
             pcidevs_lock();
             ret = pt_irq_create_bind(d, bind);
             pcidevs_unlock();
+
+            if ( ret < 0 )
+                printk(XENLOG_G_ERR "pt_irq_create_bind failed (%ld) for %pd\n",
+                       ret, d);
         }
-        if ( ret < 0 )
-            printk(XENLOG_G_ERR "pt_irq_create_bind failed (%ld) for dom%d\n",
-                   ret, d->domain_id);
+        else
+            ret = -ESRCH;
+
+        read_unlock(&currd->caps_lock);
         break;
     }
 
@@ -609,23 +617,26 @@ long arch_do_domctl(
         if ( !is_hvm_domain(d) )
             break;
 
-        ret = -EPERM;
-        if ( irq <= 0 || !irq_access_permitted(currd, irq) )
-            break;
-
         ret = xsm_unbind_pt_irq(XSM_HOOK, d, bind);
         if ( ret )
             break;
 
-        if ( is_iommu_enabled(d) )
+        read_lock(&currd->caps_lock);
+
+        if ( !irq_access_permitted(currd, irq) )
+            ret = -EPERM;
+        else if ( is_iommu_enabled(d) )
         {
             pcidevs_lock();
             ret = pt_irq_destroy_bind(d, bind);
             pcidevs_unlock();
+
+            if ( ret < 0 )
+                printk(XENLOG_G_ERR "pt_irq_destroy_bind failed (%ld) for %pd\n",
+                       ret, d);
         }
-        if ( ret < 0 )
-            printk(XENLOG_G_ERR "pt_irq_destroy_bind failed (%ld) for dom%d\n",
-                   ret, d->domain_id);
+
+        read_unlock(&currd->caps_lock);
         break;
     }
 

xen/common/domctl.c

@@ -700,6 +700,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = -EINVAL;
             break;
         }
+
+        iocaps_double_lock(d, true);
+
         irq = pirq_access_permitted(current->domain, pirq);
         if ( !irq || xsm_irq_permission(XSM_HOOK, d, irq, allow) )
             ret = -EPERM;
@@ -707,6 +710,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = irq_permit_access(d, irq);
         else
             ret = irq_deny_access(d, irq);
+
+        iocaps_double_unlock(d, true);
         break;
     }
 #endif