[ovs-dev] [PATCH] datapath-windows: Solved BSOD when uninstalling the driver (race condition)

[Sorin Vinturis]

The BSOD occurred because the FilterAttach routine released the switch
context, while there were IRPs in processing.

The solution was to add a reference count in the switch context to prevent
premature deallocation of the switch context structure.

Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
Reported-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/58
---
 datapath-windows/ovsext/Datapath.c | 10 +++++----
 datapath-windows/ovsext/Switch.c   | 44 ++++++++++++++++++++++++++++++++++++++
 datapath-windows/ovsext/Switch.h   | 12 ++++++++++-
 3 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 8eb13f1..8b561a3 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -699,10 +699,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
     outputBufferLen = irpSp->Parameters.DeviceIoControl.OutputBufferLength;
     inputBuffer = irp->AssociatedIrp.SystemBuffer;
 
-    /* Check if the extension is enabled. */
-    if (NULL == gOvsSwitchContext) {
-        status = STATUS_DEVICE_NOT_READY;
-        goto done;
+    if (!OvsAcquireSwitchContext(gOvsSwitchContext)) {
+        status = STATUS_NOT_FOUND;
+        goto exit;
     }
 
     /* Concurrent netlink operations are not supported. */
@@ -874,6 +873,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
     status = InvokeNetlinkCmdHandler(&usrParamsCtx, nlFamilyOps, &replyLen);
 
 done:
+    OvsReleaseSwitchContext(gOvsSwitchContext);
+
+exit:
     KeMemoryBarrier();
     instance->inUse = 0;
 
diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index a228d8e..8366944 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -420,6 +420,9 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
     switchContext->isActivateFailed = FALSE;
     switchContext->dpNo = OVS_DP_NUMBER;
     ovsTimeIncrementPerTick = KeQueryTimeIncrement() / 10000;
+
+    switchContext->refCount = 1;
+
     OVS_LOG_TRACE("Exit: Succesfully initialized switchContext: %p",
                   switchContext);
     return NDIS_STATUS_SUCCESS;
@@ -428,6 +431,12 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 static VOID
 OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 {
+    OvsReleaseSwitchContext(switchContext);
+}
+
+VOID
+OvsDeleteSwitchContext(POVS_SWITCH_CONTEXT switchContext)
+{
     OVS_LOG_TRACE("Enter: Delete switchContext:%p", switchContext);
 
     /* We need to do cleanup for tunnel port here. */
@@ -447,9 +456,44 @@ OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
     switchContext->pidHashArray = NULL;
     OvsDeleteFlowTable(&switchContext->datapath);
     OvsCleanupBufferPool(switchContext);
+    switchContext->refCount = 0;
     OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext);
 }
 
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext)
+{
+    if (1 == InterlockedCompareExchange(
+        (LONG volatile *)&switchContext->refCount, 0, 1)) {
+        OvsDeleteSwitchContext(switchContext);
+    }
+    else {
+        InterlockedDecrement((LONG volatile *)&switchContext->refCount);
+    }
+}
+
+BOOLEAN
+OvsAcquireSwitchContext(POVS_SWITCH_CONTEXT switchContext)
+{
+    BOOLEAN ret = FALSE;
+
+    do {
+        if (NULL == switchContext) {
+            break;
+        }
+
+        if (0 == InterlockedCompareExchange(
+            (LONG volatile *)&switchContext->refCount, 0, 0)) {
+            break;
+        }
+
+        InterlockedIncrement((LONG volatile *)&switchContext->refCount);
+        ret = TRUE;
+    } while (!ret);
+
+    return ret;
+}
+
 /*
  * --------------------------------------------------------------------------
  *  This function activates the switch by initializing it with all the runtime
diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
index 7960072..25db636 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -179,6 +179,12 @@ typedef struct _OVS_SWITCH_CONTEXT
     volatile LONG pendingOidCount;
 
     OVS_NBL_POOL            ovsPool;
+
+    /*
+     * Reference count used to prevent premature deallocation of the switch
+     * context.
+     */
+    INT32                   refCount;
 } OVS_SWITCH_CONTEXT, *POVS_SWITCH_CONTEXT;
 
 
@@ -202,7 +208,6 @@ OvsAcquireDatapathWrite(OVS_DATAPATH *datapath,
                            dispatch ? NDIS_RWL_AT_DISPATCH_LEVEL : 0);
 }
 
-
 static __inline VOID
 OvsReleaseDatapath(OVS_DATAPATH *datapath,
                    LOCK_STATE_EX *lockState)
@@ -211,6 +216,11 @@ OvsReleaseDatapath(OVS_DATAPATH *datapath,
     NdisReleaseRWLock(datapath->lock, lockState);
 }
 
+BOOLEAN
+OvsAcquireSwitchContext(POVS_SWITCH_CONTEXT switchContext);
+
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext);
 
 PVOID OvsGetExternalVport();
 
-- 
1.9.0.msysgit.0