All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH v3] x86: irq: Do not BUG_ON multiple unbind calls for shared pirqs
@ 2020-01-29  9:28 Varad Gautam
  2020-01-29 10:08 ` Julien Grall
  2020-01-29 10:30 ` Roger Pau Monné
  0 siblings, 2 replies; 7+ messages in thread
From: Varad Gautam @ 2020-01-29  9:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Varad Gautam, Julien Grall, Roger Pau Monné

XEN_DOMCTL_destroydomain creates a continuation if domain_kill -ERESTARTS.
In that scenario, it is possible to receive multiple _pirq_guest_unbind
calls for the same pirq from domain_kill, if the pirq has not yet been
removed from the domain's pirq_tree, as:
  domain_kill()
    -> domain_relinquish_resources()
      -> pci_release_devices()
        -> pci_clean_dpci_irq()
          -> pirq_guest_unbind()
            -> __pirq_guest_unbind()

For a shared pirq (nr_guests > 1), the first call would zap the current
domain from the pirq's guests[] list, but the action handler is never freed
as there are other guests using this pirq. As a result, on the second call,
__pirq_guest_unbind searches for the current domain which has been removed
from the guests[] list, and hits a BUG_ON.

Make __pirq_guest_unbind safe to be called multiple times by letting xen
continue if a shared pirq has already been unbound from this guest. The
PIRQ will be cleaned up from the domain's pirq_tree during the destruction
in complete_domain_destroy anyways.

Signed-off-by: Varad Gautam <vrd@amazon.de>
Reviewed-by: Paul Durrant <paul@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

v2: Split the check on action->nr_guests > 0 and make it an ASSERT.
v3: Style fixups.
---
 xen/arch/x86/irq.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 310ac00..4b172eb 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1681,7 +1681,20 @@ static irq_guest_action_t *__pirq_guest_unbind(
 
     for ( i = 0; (i < action->nr_guests) && (action->guest[i] != d); i++ )
         continue;
-    BUG_ON(i == action->nr_guests);
+    if ( i == action->nr_guests )
+    {
+        ASSERT(action->nr_guests > 0);
+        /*
+         * In case the pirq was shared, unbound for this domain in an earlier
+         * call, but still existed on the domain's pirq_tree, we still reach
+         * here if there are any later unbind calls on the same pirq. Return
+         * if such an unbind happens.
+         */
+        if ( action->shareable )
+            return NULL;
+        BUG();
+    }
+
     memmove(&action->guest[i], &action->guest[i+1],
             (action->nr_guests-i-1) * sizeof(action->guest[0]));
     action->nr_guests--;
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-06 15:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-29  9:28 [Xen-devel] [PATCH v3] x86: irq: Do not BUG_ON multiple unbind calls for shared pirqs Varad Gautam
2020-01-29 10:08 ` Julien Grall
2020-01-29 10:30 ` Roger Pau Monné
2020-01-29 11:47   ` Jan Beulich
2020-03-05  9:36     ` Jan Beulich
2020-03-05 17:37       ` Durrant, Paul
2020-03-06 15:07         ` Durrant, Paul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.