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 4de45904d098bdecbc77743d795b05892c433e66..05d467e1d7389e541c9c022f95f47f41612b3ec1 100644 (file)
--- a/include/evtchn_interface.h
+++ b/include/evtchn_interface.h
@@ -189,17 +189,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
     );
 
@@ -312,7 +334,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;
 };
@@ -330,12 +352,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
@@ -346,7 +387,7 @@ typedef struct _XENBUS_EVTCHN_INTERFACE_V6 XENBUS_EVTCHN_INTERFACE, *PXENBUS_EVT
 #endif  // _WINDLL
 
 #define XENBUS_EVTCHN_INTERFACE_VERSION_MIN 1
-#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 6d2e408528ee94e5cb7569d84ff6fa9317456524..99032934c20882ed0c92c1c52e165081726fd629 100644 (file)
--- a/include/revision.h
+++ b/include/revision.h
@@ -48,6 +48,7 @@
     DEFINE_REVISION(0x08000009,  1,  2,  4,  1,  1,  1,  1,  1,  1,  1),    \
     DEFINE_REVISION(0x0800000A,  1,  2,  5,  1,  1,  1,  1,  1,  1,  1),    \
     DEFINE_REVISION(0x0800000B,  1,  2,  5,  1,  2,  1,  1,  2,  1,  1),    \
-    DEFINE_REVISION(0x0800000C,  1,  2,  6,  1,  2,  1,  1,  2,  1,  1)
+    DEFINE_REVISION(0x0800000C,  1,  2,  6,  1,  2,  1,  1,  2,  1,  1),    \
+    DEFINE_REVISION(0x0800000D,  1,  2,  7,  1,  2,  1,  1,  2,  1,  1)
 
 #endif  // _REVISION_H

diff --git a/src/xenbus/evtchn.c b/src/xenbus/evtchn.c
index f965f5bc39b425dacef9d63414ac0dccc7bd97d0..91afa62cce35c5bc068669a81ab0c5459f08b7aa 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);
         }
     }
 }
@@ -1693,7 +1724,7 @@ static struct _XENBUS_EVTCHN_INTERFACE_V5 EvtchnInterfaceVersion5 = {
     EvtchnUnmask,
     EvtchnSendVersion1,
     EvtchnTrigger,
-    EvtchnWait,
+    EvtchnWaitVersion5,
     EvtchnGetPort,
     EvtchnClose,
 };
@@ -1707,6 +1738,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,
@@ -1922,6 +1968,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();
     }