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

[Ankur Sharma]

Hi Sorin,
Can you please provide some more details of the crash?

Either of following data would be helpful in review.
a. Repro steps.
b. Backtrace.

Thanks.

Regards,
Ankur
________________________________________
From: dev <dev-bounces at openvswitch.org> on behalf of Sorin Vinturis <svinturis at cloudbasesolutions.com>
Sent: Thursday, October 30, 2014 7:32 AM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Avoid BSOD when switch context     is NULL

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
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=jzzqQhwO0BiBl12LDMUQlqlyYOHlyd0d1PMyWNDa70Y%3D%0A&s=eb1e0bf79ccce74aa89343542872a361e6fb70e5f5db5f9999a3372a9c735280