[xen staging] evtchn: avoid race in get_xen_consumer()

[patchbot@xxxxxxx]

commit dcbd1d8ae527f3632eb9a2804ad03e0451918b62
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Fri Oct 23 10:07:56 2020 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Oct 23 10:07:56 2020 +0200

    evtchn: avoid race in get_xen_consumer()
    
    There's no global lock around the updating of this global piece of data.
    Make use of cmpxchgptr() to avoid two entities racing with their
    updates.
    
    While touching the functionality, mark xen_consumers[] read-mostly (or
    else the if() condition could use the result of cmpxchgptr(), writing to
    the slot unconditionally).
    
    The use of cmpxchgptr() here points out (by way of clang warning about
    it) that its original use of const was slightly wrong. Adjust the
    placement, or else undefined behavior of const qualifying a function
    type will result.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/common/event_channel.c   |  6 ++++--
 xen/include/asm-x86/system.h |  7 -------
 xen/include/xen/lib.h        | 11 +++++++++++
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index e365b5498f..091afa8bf5 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -57,7 +57,8 @@
  * with a pointer, we stash them dynamically in a small lookup array which
  * can be indexed by a small integer.
  */
-static xen_event_channel_notification_t xen_consumers[NR_XEN_CONSUMERS];
+static xen_event_channel_notification_t __read_mostly
+    xen_consumers[NR_XEN_CONSUMERS];
 
 /* Default notification action: wake up from wait_on_xen_event_channel(). */
 static void default_xen_notification_fn(struct vcpu *v, unsigned int port)
@@ -80,8 +81,9 @@ static uint8_t 
get_xen_consumer(xen_event_channel_notification_t fn)
 
     for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
     {
+        /* Use cmpxchgptr() in lieu of a global lock. */
         if ( xen_consumers[i] == NULL )
-            xen_consumers[i] = fn;
+            cmpxchgptr(&xen_consumers[i], NULL, fn);
         if ( xen_consumers[i] == fn )
             break;
     }
diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index 630909965e..65e63de69a 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -148,13 +148,6 @@ static always_inline unsigned long cmpxchg_local_(
     return prev;
 }
 
-#define cmpxchgptr(ptr,o,n) ({                                          \
-    const __typeof__(**(ptr)) *__o = (o);                               \
-    __typeof__(**(ptr)) *__n = (n);                                     \
-    ((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)__o,            \
-                                   (unsigned long)__n,sizeof(*(ptr)))); \
-})
-
 /*
  * Undefined symbol to cause link failure if a wrong size is used with
  * arch_fetch_and_add().
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 076bcfb67d..1983bd6b86 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -178,6 +178,17 @@ unsigned long long parse_size_and_unit(const char *s, 
const char **ps);
 
 uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
 
+/*
+ * A slightly more typesafe variant of cmpxchg() when the entities dealt with
+ * are pointers.
+ */
+#define cmpxchgptr(ptr, o, n) ({                                        \
+    __typeof__(**(ptr)) *const o_ = (o);                                \
+    __typeof__(**(ptr)) *n_ = (n);                                      \
+    ((__typeof__(*(ptr)))__cmpxchg(ptr, (unsigned long)o_,              \
+                                   (unsigned long)n_, sizeof(*(ptr)))); \
+})
+
 #define TAINT_SYNC_CONSOLE              (1u << 0)
 #define TAINT_MACHINE_CHECK             (1u << 1)
 #define TAINT_ERROR_INJECT              (1u << 2)
--
generated by git-patchbot for /home/xen/git/xen.git#staging