All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
Date: Tue, 20 Oct 2020 16:08:13 +0200	[thread overview]
Message-ID: <9ecafa4d-db5b-20a2-3a9d-6a6cda91252c@suse.com> (raw)
In-Reply-To: <19babf20-3649-5c63-44a9-7edfa81835aa@suse.com>

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@suse.com>
---
v2: Use (and hence generalize) cmpxchgptr(). Add comment. Expand /
    adjust description.

--- 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_even
 
     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;
     }
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -148,13 +148,6 @@ static always_inline unsigned long cmpxc
     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().
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -178,6 +178,17 @@ unsigned long long parse_size_and_unit(c
 
 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)



  reply	other threads:[~2020-10-20 14:08 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 14:06 [PATCH v2 0/8] evtchn: recent XSAs follow-on Jan Beulich
2020-10-20 14:08 ` Jan Beulich [this message]
2020-10-21 15:46   ` [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer() Roger Pau Monné
2020-10-22  7:33     ` Jan Beulich
2020-10-22  8:11       ` Roger Pau Monné
2020-10-22  8:15         ` Jan Beulich
2020-10-22  8:29           ` Roger Pau Monné
2020-10-22  8:56             ` Jan Beulich
2020-10-22  9:25               ` Roger Pau Monné
2020-10-22  9:21   ` Roger Pau Monné
2020-10-30 10:15   ` Julien Grall
2020-10-20 14:08 ` [PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one Jan Beulich
2020-10-21 16:00   ` Roger Pau Monné
2020-10-30 10:21   ` Julien Grall
2020-10-30 10:42     ` Jan Beulich
2020-10-30 10:44       ` Julien Grall
2020-10-20 14:09 ` [PATCH v2 3/8] evtchn: rename and adjust guest_enabled_event() Jan Beulich
2020-10-22 10:28   ` Roger Pau Monné
2020-10-20 14:09 ` [PATCH v2 4/8] evtchn: let evtchn_set_priority() acquire the per-channel lock Jan Beulich
2020-10-22 11:17   ` Roger Pau Monné
2020-10-22 13:34     ` Jan Beulich
2020-10-20 14:10 ` [PATCH v2 5/8] evtchn: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq() Jan Beulich
2020-10-22 16:00   ` Roger Pau Monné
2020-10-22 16:17     ` Jan Beulich
2020-10-30 10:38       ` Julien Grall
2020-10-30 10:49         ` Jan Beulich
2020-10-30 10:57           ` Julien Grall
2020-10-30 11:15             ` Jürgen Groß
2020-10-30 11:55               ` Jan Beulich
2020-10-30 12:27                 ` Jürgen Groß
2020-10-30 12:52                   ` Jan Beulich
2020-10-30 13:02                     ` Jürgen Groß
2020-10-30 13:38                       ` Jan Beulich
2020-10-30 13:43                         ` Jürgen Groß
2020-10-20 14:10 ` [PATCH v2 6/8] evtchn: convert vIRQ lock to an r/w one Jan Beulich
2020-10-30 10:57   ` Julien Grall
2020-10-30 12:00     ` Jan Beulich
2020-10-30 12:08       ` Julien Grall
2020-10-30 12:25         ` Jan Beulich
2020-10-30 12:46           ` Julien Grall
2020-10-20 14:11 ` [PATCH v2 7/8] evtchn: convert domain event " Jan Beulich
2020-10-20 14:13 ` [PATCH RFC v2 8/8] evtchn: don't call Xen consumer callback with per-channel lock held Jan Beulich
2020-11-03 10:17   ` Isaila Alexandru
2020-11-03 14:54     ` Tamas K Lengyel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ecafa4d-db5b-20a2-3a9d-6a6cda91252c@suse.com \
    --to=jbeulich@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=julien@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.