datapath-windows: Solved BSOD when uninstalling the driver (race condition)

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 to prevent premature deallocation of the
global switch context structure, gOvsSwitchContext.

Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
Reported-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/58
Acked-by: Eitan Eliahu <eliahue@vmware.com>
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index e566ea9..fea7d3a 100644 (file)
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -717,8 +717,13 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 
     /* Check if the extension is enabled. */
     if (NULL == gOvsSwitchContext) {
-        status = STATUS_DEVICE_NOT_READY;
-        goto done;
+        status = STATUS_NOT_FOUND;
+        goto exit;
+    }
+
+    if (!OvsAcquireSwitchContext()) {
+        status = STATUS_NOT_FOUND;
+        goto exit;
     }
 
     /* Concurrent netlink operations are not supported. */
@@ -908,6 +913,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 61a4531..4f4591f 100644 (file)
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -42,6 +42,12 @@ extern PNDIS_SPIN_LOCK gOvsCtrlLock;
 extern NDIS_HANDLE gOvsExtDriverHandle;
 extern NDIS_HANDLE gOvsExtDriverObject;
 
+/*
+ * Reference count used to prevent premature deallocation of the global switch
+ * context structure, gOvsSwitchContext.
+ */
+volatile LONG      gOvsSwitchContextRefCount = 1;
+
 static NDIS_STATUS OvsCreateSwitch(NDIS_HANDLE ndisFilterHandle,
                                    POVS_SWITCH_CONTEXT *switchContextOut);
 static NDIS_STATUS OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext);
@@ -423,6 +429,7 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
     switchContext->isActivateFailed = FALSE;
     switchContext->dpNo = OVS_DP_NUMBER;
     ovsTimeIncrementPerTick = KeQueryTimeIncrement() / 10000;
+
     OVS_LOG_TRACE("Exit: Succesfully initialized switchContext: %p",
                   switchContext);
     return NDIS_STATUS_SUCCESS;
@@ -430,6 +437,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);
 
@@ -457,6 +470,49 @@ OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
     OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext);
 }
 
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext)
+{
+    LONG ref = 0;
+    LONG newRef = 0;
+    LONG icxRef = 0;
+
+    do {
+        ref = gOvsSwitchContextRefCount;
+        newRef = (0 == ref) ? 0 : ref - 1;
+        icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
+                                            newRef,
+                                            ref);
+    } while (icxRef != ref);
+
+    if (ref == 1) {
+        OvsDeleteSwitchContext(switchContext);
+    }
+}
+
+BOOLEAN
+OvsAcquireSwitchContext(VOID)
+{
+    LONG ref = 0;
+    LONG newRef = 0;
+    LONG icxRef = 0;
+    BOOLEAN ret = FALSE;
+
+    do {
+        ref = gOvsSwitchContextRefCount;
+        newRef = (0 == ref) ? 0 : ref + 1;
+        icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
+                                            newRef,
+                                            ref);
+    } while (icxRef != ref);
+
+    if (ref != 0) {
+        ret = TRUE;
+    }
+
+    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..6ec34e1 100644 (file)
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -202,7 +202,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 +210,11 @@ OvsReleaseDatapath(OVS_DATAPATH *datapath,
     NdisReleaseRWLock(datapath->lock, lockState);
 }
 
+BOOLEAN
+OvsAcquireSwitchContext(VOID);
+
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext);
 
 PVOID OvsGetExternalVport();