Update XENBUS_EVTCHN Wait method to avoid races

Use of the XENBUS_EVTCHN method is prone to races in that a client
usually wants to check that something that should be triggered by an
event and, if it has not occurred, wait for the next event.
The problem is that the client may makes its check, the event then occurs,
and then the client waits. Thus the event is missed and the client only
wakes up when the timeout expires.

This patch changes the Wait method to take an explicit event count to wait
for, and adds a method to sample the current event count. A client can
then avoid a race as described above by sampling the event count first,
making its check and then waiting for the event count to increment by one.
If the event occurred between the check and the wait, the wait will now
return immediately.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

diff --git a/include/evtchn_interface.h b/include/evtchn_interface.h
index 4e696c8b1f98ddaa2136cc5c851889ef1f54fa76..310850bab2582584179d71a02d2008ae7509fd00 100644 (file)
--- a/include/evtchn_interface.h
+++ b/include/evtchn_interface.h
@@ -175,17 +175,39 @@ typedef VOID
     IN  PXENBUS_EVTCHN_CHANNEL  Channel
     );
 
+/*! \typedef XENBUS_EVTCHN_GET_COUNT
+    \brief Get the number of events received by the channel since it was opened
+
+    \param Interface The interface header
+    \param Channel The channel handle
+    \return The number of events
+*/
+typedef ULONG
+(*XENBUS_EVTCHN_GET_COUNT)(
+    IN  PINTERFACE              Interface,
+    IN  PXENBUS_EVTCHN_CHANNEL  Channel
+    );
+
+typedef NTSTATUS
+(*XENBUS_EVTCHN_WAIT_V5)(
+    IN  PINTERFACE              Interface,
+    IN  PXENBUS_EVTCHN_CHANNEL  Channel,
+    IN  PLARGE_INTEGER          Timeout OPTIONAL
+    );
+
 /*! \typedef XENBUS_EVTCHN_WAIT
-    \brief Wait for an event to the local end of the channel
+    \brief Wait for events to the local end of the channel
 
     \param Interface The interface header
     \param Channel The channel handle
+    \param Count The event count to wait for
     \param Timeout An optional timeout value (similar to KeWaitForSingleObject(), but non-zero values are allowed at DISPATCH_LEVEL).
 */
 typedef NTSTATUS
 (*XENBUS_EVTCHN_WAIT)(
     IN  PINTERFACE              Interface,
     IN  PXENBUS_EVTCHN_CHANNEL  Channel,
+    IN  ULONG                   Count,
     IN  PLARGE_INTEGER          Timeout OPTIONAL
     );
 
@@ -248,7 +270,7 @@ struct _XENBUS_EVTCHN_INTERFACE_V5 {
     XENBUS_EVTCHN_UNMASK    EvtchnUnmask;
     XENBUS_EVTCHN_SEND_V1   EvtchnSendVersion1;
     XENBUS_EVTCHN_TRIGGER   EvtchnTrigger;
-    XENBUS_EVTCHN_WAIT      EvtchnWait;
+    XENBUS_EVTCHN_WAIT_V5   EvtchnWaitVersion5;
     XENBUS_EVTCHN_GET_PORT  EvtchnGetPort;
     XENBUS_EVTCHN_CLOSE     EvtchnClose;
 };
@@ -266,12 +288,31 @@ struct _XENBUS_EVTCHN_INTERFACE_V6 {
     XENBUS_EVTCHN_UNMASK    EvtchnUnmask;
     XENBUS_EVTCHN_SEND      EvtchnSend;
     XENBUS_EVTCHN_TRIGGER   EvtchnTrigger;
+    XENBUS_EVTCHN_WAIT_V5   EvtchnWaitVersion5;
+    XENBUS_EVTCHN_GET_PORT  EvtchnGetPort;
+    XENBUS_EVTCHN_CLOSE     EvtchnClose;
+};
+
+/*! \struct _XENBUS_EVTCHN_INTERFACE_V7
+    \brief EVTCHN interface version 7
+    \ingroup interfaces
+*/
+struct _XENBUS_EVTCHN_INTERFACE_V7 {
+    INTERFACE               Interface;
+    XENBUS_EVTCHN_ACQUIRE   EvtchnAcquire;
+    XENBUS_EVTCHN_RELEASE   EvtchnRelease;
+    XENBUS_EVTCHN_OPEN      EvtchnOpen;
+    XENBUS_EVTCHN_BIND      EvtchnBind;
+    XENBUS_EVTCHN_UNMASK    EvtchnUnmask;
+    XENBUS_EVTCHN_SEND      EvtchnSend;
+    XENBUS_EVTCHN_TRIGGER   EvtchnTrigger;
+    XENBUS_EVTCHN_GET_COUNT EvtchnGetCount;
     XENBUS_EVTCHN_WAIT      EvtchnWait;
     XENBUS_EVTCHN_GET_PORT  EvtchnGetPort;
     XENBUS_EVTCHN_CLOSE     EvtchnClose;
 };
 
-typedef struct _XENBUS_EVTCHN_INTERFACE_V6 XENBUS_EVTCHN_INTERFACE, *PXENBUS_EVTCHN_INTERFACE;
+typedef struct _XENBUS_EVTCHN_INTERFACE_V7 XENBUS_EVTCHN_INTERFACE, *PXENBUS_EVTCHN_INTERFACE;
 
 /*! \def XENBUS_EVTCHN
     \brief Macro at assist in method invocation
@@ -282,7 +323,7 @@ typedef struct _XENBUS_EVTCHN_INTERFACE_V6 XENBUS_EVTCHN_INTERFACE, *PXENBUS_EVT
 #endif  // _WINDLL
 
 #define XENBUS_EVTCHN_INTERFACE_VERSION_MIN 4
-#define XENBUS_EVTCHN_INTERFACE_VERSION_MAX 6
+#define XENBUS_EVTCHN_INTERFACE_VERSION_MAX 7
 
 #endif  // _XENBUS_EVTCHN_INTERFACE_H
 

diff --git a/include/revision.h b/include/revision.h
index 2aa7dc57a7d70647812335121a5f389a6475db0b..bc73be23da012240c4a0c314c0fbddcf553f3b42 100644 (file)
--- a/include/revision.h
+++ b/include/revision.h
@@ -51,6 +51,7 @@
     DEFINE_REVISION(0x0800000A,  1,  2,  5,  1,  1,  1,  1,  1,  1,  0,  1), \
     DEFINE_REVISION(0x0800000B,  1,  2,  5,  1,  2,  1,  1,  2,  1,  0,  1), \
     DEFINE_REVISION(0x09000000,  1,  2,  5,  1,  2,  1,  1,  2,  1,  0,  1), \
-    DEFINE_REVISION(0x09000001,  1,  2,  6,  1,  2,  1,  1,  2,  1,  1,  1)
+    DEFINE_REVISION(0x09000001,  1,  2,  6,  1,  2,  1,  1,  2,  1,  1,  1), \
+    DEFINE_REVISION(0x09000002,  1,  2,  7,  1,  2,  1,  1,  2,  1,  1,  1)
 
 #endif  // _REVISION_H

diff --git a/src/xenbus/evtchn.c b/src/xenbus/evtchn.c
index 7546f1923e93b0817e5a70eeeac7f8db39c5aca4..f9f95c9b9136c935b0f4e885949d7fb17505dc8b 100644 (file)
--- a/src/xenbus/evtchn.c
+++ b/src/xenbus/evtchn.c
@@ -81,7 +81,7 @@ struct _XENBUS_EVTCHN_CHANNEL {
     PKSERVICE_ROUTINE           Callback;
     PVOID                       Argument;
     BOOLEAN                     Active; // Must be tested at >= DISPATCH_LEVEL
-    ULONG                       Events;
+    ULONG                       Count;
     XENBUS_EVTCHN_TYPE          Type;
     XENBUS_EVTCHN_PARAMETERS    Parameters;
     BOOLEAN                     Mask;
@@ -401,7 +401,7 @@ EvtchnReap(
 
     Trace("%u\n", LocalPort);
 
-    Channel->Events = 0;
+    Channel->Count = 0;
 
     ASSERT(Channel->Closed);
     Channel->Closed = FALSE;
@@ -507,7 +507,7 @@ EvtchnPoll(
 
         KeMemoryBarrier();
         if (!Channel->Closed) {
-            Channel->Events++;
+            Channel->Count++;
 
             RemoveEntryList(&Channel->PendingListEntry);
             InitializeListHead(&Channel->PendingListEntry);
@@ -898,15 +898,26 @@ EvtchnGetPort(
     return Channel->LocalPort;
 }
 
+static ULONG
+EvtchnGetCount(
+    IN  PINTERFACE              Interface,
+    IN  PXENBUS_EVTCHN_CHANNEL  Channel
+    )
+{
+    UNREFERENCED_PARAMETER(Interface);
+
+    return Channel->Count;
+}
+
 static NTSTATUS
 EvtchnWait(
     IN  PINTERFACE              Interface,
     IN  PXENBUS_EVTCHN_CHANNEL  Channel,
+    IN  ULONG                   Count,
     IN  PLARGE_INTEGER          Timeout
     )
 {
     KIRQL                       Irql;
-    ULONG                       Events;
     LARGE_INTEGER               Start;
     NTSTATUS                    status;
 
@@ -915,14 +926,13 @@ EvtchnWait(
     ASSERT3U(KeGetCurrentIrql(), <=, DISPATCH_LEVEL);
     KeRaiseIrql(DISPATCH_LEVEL, &Irql); // Prevent suspend
 
-    Events = Channel->Events;
-    KeMemoryBarrier();
-
     KeQuerySystemTime(&Start);
 
     for (;;) {
+        KeMemoryBarrier();
+
         status = STATUS_SUCCESS;
-        if (Channel->Events != Events)
+        if ((LONG64)Count - (LONG64)Channel->Count <= 0)
             break;
 
         if (Timeout != NULL) {
@@ -950,14 +960,35 @@ EvtchnWait(
         }
 
         _mm_pause();
-        KeMemoryBarrier();
     }
 
+    if (status == STATUS_TIMEOUT)
+        Info("TIMED OUT: Count = %08x Channel->Count = %08x\n",
+             Count,
+             Channel->Count);
+
     KeLowerIrql(Irql);
 
     return status;
 }
 
+static NTSTATUS
+EvtchnWaitVersion5(
+    IN  PINTERFACE              Interface,
+    IN  PXENBUS_EVTCHN_CHANNEL  Channel,
+    IN  PLARGE_INTEGER          Timeout
+    )
+{
+    ULONG                       Count;
+
+    Count = EvtchnGetCount(Interface, Channel);
+
+    return EvtchnWait(Interface,
+                      Channel,
+                      Count + 1,
+                      Timeout);
+}
+
 static
 _Function_class_(KSERVICE_ROUTINE)
 __drv_requiresIRQL(HIGH_LEVEL)
@@ -1369,8 +1400,8 @@ EvtchnDebugCallback(
 
             XENBUS_DEBUG(Printf,
                          &Context->DebugInterface,
-                         "Events = %lu\n",
-                         Channel->Events);
+                         "Count = %lu\n",
+                         Channel->Count);
         }
     }
 }
@@ -1655,7 +1686,7 @@ static struct _XENBUS_EVTCHN_INTERFACE_V5 EvtchnInterfaceVersion5 = {
     EvtchnUnmask,
     EvtchnSendVersion1,
     EvtchnTrigger,
-    EvtchnWait,
+    EvtchnWaitVersion5,
     EvtchnGetPort,
     EvtchnClose,
 };
@@ -1669,6 +1700,21 @@ static struct _XENBUS_EVTCHN_INTERFACE_V6 EvtchnInterfaceVersion6 = {
     EvtchnUnmask,
     EvtchnSend,
     EvtchnTrigger,
+    EvtchnWaitVersion5,
+    EvtchnGetPort,
+    EvtchnClose,
+};
+
+static struct _XENBUS_EVTCHN_INTERFACE_V7 EvtchnInterfaceVersion7 = {
+    { sizeof (struct _XENBUS_EVTCHN_INTERFACE_V7), 7, NULL, NULL, NULL },
+    EvtchnAcquire,
+    EvtchnRelease,
+    EvtchnOpen,
+    EvtchnBind,
+    EvtchnUnmask,
+    EvtchnSend,
+    EvtchnTrigger,
+    EvtchnGetCount,
     EvtchnWait,
     EvtchnGetPort,
     EvtchnClose,
@@ -1833,6 +1879,23 @@ EvtchnGetInterface(
         status = STATUS_SUCCESS;
         break;
     }
+    case 7: {
+        struct _XENBUS_EVTCHN_INTERFACE_V7  *EvtchnInterface;
+
+        EvtchnInterface = (struct _XENBUS_EVTCHN_INTERFACE_V7 *)Interface;
+
+        status = STATUS_BUFFER_OVERFLOW;
+        if (Size < sizeof (struct _XENBUS_EVTCHN_INTERFACE_V7))
+            break;
+
+        *EvtchnInterface = EvtchnInterfaceVersion7;
+
+        ASSERT3U(Interface->Version, ==, Version);
+        Interface->Context = Context;
+
+        status = STATUS_SUCCESS;
+        break;
+    }
     default:
         status = STATUS_NOT_SUPPORTED;
         break;

diff --git a/src/xenbus/store.c b/src/xenbus/store.c
index 86bbcb8cdb79cc5882081357b2e938014876074f..101dc40259211084b600b15a4ee44b76857c9bee 100644 (file)
--- a/src/xenbus/store.c
+++ b/src/xenbus/store.c
@@ -900,6 +900,7 @@ StoreSubmitRequest(
 {
     PXENBUS_STORE_RESPONSE      Response;
     KIRQL                       Irql;
+    ULONG                       Count;
     LARGE_INTEGER               Timeout;
 
     ASSERT3U(Request->State, ==, XENBUS_STORE_REQUEST_PREPARED);
@@ -913,6 +914,11 @@ StoreSubmitRequest(
     InsertTailList(&Context->SubmittedList, &Request->ListEntry);
 
     Request->State = XENBUS_STORE_REQUEST_SUBMITTED;
+
+    Count = XENBUS_EVTCHN(GetCount,
+                          &Context->EvtchnInterface,
+                          Context->Channel);
+
     StorePollLocked(Context);
     KeMemoryBarrier();
 
@@ -924,10 +930,15 @@ StoreSubmitRequest(
         status = XENBUS_EVTCHN(Wait,
                                &Context->EvtchnInterface,
                                Context->Channel,
+                               Count + 1,
                                &Timeout);
         if (status == STATUS_TIMEOUT)
             Warning("TIMED OUT\n");
 
+        Count = XENBUS_EVTCHN(GetCount,
+                              &Context->EvtchnInterface,
+                              Context->Channel);
+
         StorePollLocked(Context);
         KeMemoryBarrier();
     }