[ovs-dev] [PATCH] datapath-windows: Avoid BSOD when switch context is NULL

Sorin Vinturis svinturis at cloudbasesolutions.com
Thu Oct 30 14:32:13 UTC 2014


In my tests I came around a BSOD that happened when trying to
access pidHashLock from the gOvsSwitchContext, which was NULL.
The stop happened in OvsAcquirePidHashLock function.

Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>

---
 datapath-windows/ovsext/BufferMgmt.c |  8 ++++++--
 datapath-windows/ovsext/Datapath.c   |  5 ++++-
 datapath-windows/ovsext/Tunnel.c     |  3 ++-
 datapath-windows/ovsext/User.c       | 30 +++++++++++++++++++++---------
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c
index e0377c1..1a68575 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -474,15 +474,19 @@ OvsAllocateVariableSizeNBL(PVOID ovsContext,
 {
     PNET_BUFFER_LIST nbl = NULL;
     POVS_SWITCH_CONTEXT context = (POVS_SWITCH_CONTEXT)ovsContext;
-    POVS_NBL_POOL ovsPool = &context->ovsPool;
+    POVS_NBL_POOL ovsPool = NULL;
     POVS_BUFFER_CONTEXT ctx;
     UINT32 realSize;
     PMDL mdl;
     NDIS_STATUS status;
     PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO info;
-    if (size == 0) {
+
+    if ((NULL == ovsContext) || (0 == size)) {
         return NULL;
     }
+
+    ovsPool = &context->ovsPool;
+
     realSize = MEM_ALIGN_SIZE(size + headRoom);
 
     mdl = OvsAllocateMDLAndData(ovsPool->ndisHandle, realSize);
diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 1b504a2..d8962bd 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -994,9 +994,10 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,
 {
     BOOLEAN writeOk;
     OVS_MESSAGE msgOutTmp;
-    OVS_DATAPATH *datapath = &ovsSwitchContext->datapath;
+    OVS_DATAPATH *datapath = NULL;
     PNL_MSG_HDR nlMsg;
 
+    ASSERT(ovsSwitchContext);
     ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && NlBufRemLen(nlBuf) >= sizeof *msgIn);
 
     msgOutTmp.nlMsg.nlmsgType = OVS_WIN_NL_DATAPATH_FAMILY_ID;
@@ -1009,6 +1010,8 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,
     msgOutTmp.genlMsg.reserved = 0;
 
     msgOutTmp.ovsHdr.dp_ifindex = ovsSwitchContext->dpNo;
+    
+    datapath = &ovsSwitchContext->datapath;
 
     writeOk = NlMsgPutHead(nlBuf, (PCHAR)&msgOutTmp, sizeof msgOutTmp);
     if (writeOk) {
diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index b55a223..fed58f1 100644
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -224,9 +224,10 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
     OvsCompletionList completionList;
     KIRQL irql;
     ULONG SendFlags = NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
-    OVS_DATAPATH *datapath = &gOvsSwitchContext->datapath;
+    OVS_DATAPATH *datapath = NULL;
 
     ASSERT(gOvsSwitchContext);
+    datapath = &gOvsSwitchContext->datapath;
 
     /* Fill the tunnel key */
     status = OvsSlowPathDecapVxlan(pNbl, &tunnelKey);
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index fc27f7d..6e84d39 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -52,13 +52,19 @@ extern NL_POLICY nlFlowKeyPolicy[];
 static __inline VOID
 OvsAcquirePidHashLock()
 {
-    NdisAcquireSpinLock(&(gOvsSwitchContext->pidHashLock));
+    if (gOvsSwitchContext)
+    {
+        NdisAcquireSpinLock(&(gOvsSwitchContext->pidHashLock));
+    }
 }
 
 static __inline VOID
 OvsReleasePidHashLock()
 {
-    NdisReleaseSpinLock(&(gOvsSwitchContext->pidHashLock));
+    if (gOvsSwitchContext)
+    {
+        NdisReleaseSpinLock(&(gOvsSwitchContext->pidHashLock));
+    }
 }
 
 
@@ -660,15 +666,21 @@ OvsGetPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid)
 {
     POVS_OPEN_INSTANCE instance;
     PLIST_ENTRY head, link;
-    UINT32 hash = OvsJhashBytes((const VOID *)&pid, sizeof(pid),
-                                OVS_HASH_BASIS);
-    head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
-    LIST_FORALL(head, link) {
-        instance = CONTAINING_RECORD(link, OVS_OPEN_INSTANCE, pidLink);
-        if (instance->pid == pid) {
-            return instance;
+    UINT32 hash = 0;
+
+    if (switchContext)
+    {
+        hash = OvsJhashBytes((const VOID *)&pid, sizeof(pid),
+            OVS_HASH_BASIS);
+        head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
+        LIST_FORALL(head, link) {
+            instance = CONTAINING_RECORD(link, OVS_OPEN_INSTANCE, pidLink);
+            if (instance->pid == pid) {
+                return instance;
+            }
         }
     }
+
     return NULL;
 }
 
-- 
1.9.0.msysgit.0



More information about the dev mailing list