pirq_cleanup_check() leaks

Its original introduction had two issues: For one the "common" part of
the checks (carried out in the macro) was inverted. And then after
removal from the radix tree the structure wasn't scheduled for freeing.
(All structures still left in the radix tree would be freed upon domain
destruction, though.)

For the freeing to be safe even if it didn't use RCU (i.e. to avoid use-
after-free), re-arrange checks/operations in evtchn_close(), such that
the pointer wouldn't be used anymore after calling pirq_cleanup_check()
(noting that unmap_domain_pirq_emuirq() itself calls the function in the
success case).

Fixes: c24536b636f2 ("replace d->nr_pirqs sized arrays with radix tree")
Fixes: 79858fee307c ("xen: fix hvm_domain_use_pirq's behavior")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 9a611c79e0241f4c8378713a45976dc2dc447dc1..017a94e31155488f29162b7d8327b171a456d9f2 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1400,6 +1400,7 @@ void (pirq_cleanup_check)(struct pirq *pirq, struct domain *d)
 
     if ( radix_tree_delete(&d->pirq_tree, pirq->pirq) != pirq )
         BUG();
+    free_pirq_struct(pirq);
 }
 
 /* Flush all ready EOIs from the top of this CPU's pending-EOI stack. */

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index a67feff98976ecf2477a3b302d06bf80194ad2d2..aac7d4b93fb2c385bda1c08ff8f07f08cf428b97 100644 (file)
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -710,9 +710,14 @@ int evtchn_close(struct domain *d1, int port1, bool guest)
             if ( !is_hvm_domain(d1) )
                 pirq_guest_unbind(d1, pirq);
             pirq->evtchn = 0;
-            pirq_cleanup_check(pirq, d1);
-            if ( is_hvm_domain(d1) && domain_pirq_to_irq(d1, pirq->pirq) > 0 )
-                unmap_domain_pirq_emuirq(d1, pirq->pirq);
+            if ( !is_hvm_domain(d1) ||
+                 domain_pirq_to_irq(d1, pirq->pirq) <= 0 ||
+                 unmap_domain_pirq_emuirq(d1, pirq->pirq) < 0 )
+                /*
+                 * The successful path of unmap_domain_pirq_emuirq() will have
+                 * called pirq_cleanup_check() already.
+                 */
+                pirq_cleanup_check(pirq, d1);
         }
         unlink_pirq_port(chn1, d1->vcpu[chn1->notify_vcpu_id]);
         break;

diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 17211f3399b7c0370101129a566421a887284b30..ee45d19af82440b561f7b09169411116641946e5 100644 (file)
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -186,7 +186,7 @@ extern struct pirq *pirq_get_info(struct domain *d, int pirq);
 void pirq_cleanup_check(struct pirq *pirq, struct domain *d);
 
 #define pirq_cleanup_check(pirq, d) \
-    ((pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
+    (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
 
 extern void pirq_guest_eoi(struct pirq *pirq);
 extern void desc_guest_eoi(struct irq_desc *desc, struct pirq *pirq);