From: Roger Pau Monné <roger.pau@citrix.com>
Date: Tue, 15 Dec 2020 12:42:16 +0000 (+0100)
Subject: x86/irq: fix infinite loop in irq_move_cleanup_interrupt
X-Git-Tag: archive/raspbian/4.16.0+51-g0941d6cb-1+rpi1~2^2~42^2~1305
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=ca85682e8c16361fdf3814c9b25a2ec3ff4f8bed;p=xen.git

x86/irq: fix infinite loop in irq_move_cleanup_interrupt

If Xen enters irq_move_cleanup_interrupt with a dynamic vector below
IRQ_MOVE_CLEANUP_VECTOR pending in IRR (0x20 or 0x21) that's also
designated for a cleanup it will enter a loop where
irq_move_cleanup_interrupt continuously sends a cleanup IPI (vector
0x22) to itself while waiting for the vector with lower priority to be
injected - which will never happen because IRQ_MOVE_CLEANUP_VECTOR
takes precedence and it's always injected first.

Fix this by making sure vectors below IRQ_MOVE_CLEANUP_VECTOR are
marked as used and thus not available for APs. Also add some logic to
assert and prevent irq_move_cleanup_interrupt from entering such an
infinite loop, albeit that should never happen given the current code.

This is XSA-356 / CVE-2020-29567.

Fixes: 3fba06ba9f8 ('x86/IRQ: re-use legacy vector ranges on APs')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index f82c93dfdc..768a8fc7c9 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -448,8 +448,15 @@ int __init init_irq_data(void)
     set_bit(HYPERCALL_VECTOR, used_vectors);
 #endif
     
-    /* IRQ_MOVE_CLEANUP_VECTOR used for clean up vectors */
-    set_bit(IRQ_MOVE_CLEANUP_VECTOR, used_vectors);
+    /*
+     * Mark vectors up to the cleanup one as used, to prevent an infinite loop
+     * invoking irq_move_cleanup_interrupt.
+     */
+    BUILD_BUG_ON(IRQ_MOVE_CLEANUP_VECTOR < FIRST_DYNAMIC_VECTOR);
+    for ( vector = FIRST_DYNAMIC_VECTOR;
+          vector <= IRQ_MOVE_CLEANUP_VECTOR;
+          vector++ )
+        __set_bit(vector, used_vectors);
 
     return 0;
 }
@@ -734,10 +741,6 @@ void irq_move_cleanup_interrupt(struct cpu_user_regs *regs)
 {
     unsigned vector, me;
 
-    /* This interrupt should not nest inside others. */
-    BUILD_BUG_ON(APIC_PRIO_CLASS(IRQ_MOVE_CLEANUP_VECTOR) !=
-                 APIC_PRIO_CLASS(FIRST_DYNAMIC_VECTOR));
-
     ack_APIC_irq();
 
     me = smp_processor_id();
@@ -781,6 +784,11 @@ void irq_move_cleanup_interrupt(struct cpu_user_regs *regs)
          */
         if ( irr & (1u << (vector % 32)) )
         {
+            if ( vector < IRQ_MOVE_CLEANUP_VECTOR )
+            {
+                ASSERT_UNREACHABLE();
+                goto unlock;
+            }
             send_IPI_self(IRQ_MOVE_CLEANUP_VECTOR);
             TRACE_3D(TRC_HW_IRQ_MOVE_CLEANUP_DELAY,
                      irq, vector, smp_processor_id());