[ovs-dev] [PATCH] datapath-windows: BSOD cause by DV due to memory leaks

[Eitan Eliahu]

Sorin thank you for fixing the memory leaks.
Minor issues:
Can you please set both the lock pointers to NULL after freeing them ?
Also, since OvsCleanupSwitchContext() reverses OvsInitSwitchContext() it would better to call OvsInitSwitchContext() as OvsDeinitSwitchContext.

Beside that LG.
Acked-by: Eitan Eliahu <eliahue at vmware.com>
Eitan




-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Monday, October 27, 2014 4:27 AM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: BSOD cause by DV due to memory leaks

If the OVS extension is enabled, Driver Verifier will issue a BSOD due to memory leaks. This issue reproduces each time and the problem is in the filter attach routine when the switch context is initialized.

Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
Reported-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
Reported-at: https://urldefense.proofpoint.com/v1/url?u=https://github.com/openvswitch/ovs-issues/issues/50&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=xKHZjHUGhbkwNXBDEoDaCab0EiVqUxWjgM2k%2BDSkvXI%3D%0A&s=d6c78a6085fa1b71b8a9fd7c5e231b0b13497fc656b6104637a8db1036876f9d
---
 datapath-windows/ovsext/Switch.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index f445500..f8e72ac 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -205,6 +205,7 @@ OvsCreateSwitch(NDIS_HANDLE ndisFilterHandle,
 
     status = OvsTunnelFilterInitialize(gOvsExtDriverObject);
     if (status != NDIS_STATUS_SUCCESS) {
+        OvsCleanupSwitchContext(switchContext);
         OvsFreeMemory(switchContext);
         goto create_switch_done;
     }
@@ -254,14 +255,18 @@ OvsExtDetach(NDIS_HANDLE filterModuleContext)  VOID  OvsDeleteSwitch(POVS_SWITCH_CONTEXT switchContext)  {
-    UINT32 dpNo = switchContext->dpNo;
+    UINT32 dpNo = (UINT32) -1;
 
     OVS_LOG_TRACE("Enter: switchContext:%p", switchContext);
 
-    OvsTunnelFilterUninitialize(gOvsExtDriverObject);
-    OvsClearAllSwitchVports(switchContext);
-    OvsCleanupSwitchContext(switchContext);
-    OvsFreeMemory(switchContext);
+    if (switchContext)
+    {
+        dpNo = switchContext->dpNo;
+        OvsTunnelFilterUninitialize(gOvsExtDriverObject);
+        OvsClearAllSwitchVports(switchContext);
+        OvsCleanupSwitchContext(switchContext);
+        OvsFreeMemory(switchContext);
+    }
     OVS_LOG_TRACE("Exit: deleted switch %p  dpNo: %d", switchContext, dpNo);  }
 
@@ -432,9 +437,13 @@ OvsCleanupSwitchContext(POVS_SWITCH_CONTEXT switchContext)
     NdisFreeRWLock(switchContext->dispatchLock);
     NdisFreeSpinLock(&(switchContext->pidHashLock));
     OvsFreeMemory(switchContext->ovsPortNameHashArray);
+    switchContext->ovsPortNameHashArray = NULL;
     OvsFreeMemory(switchContext->portIdHashArray);
+    switchContext->portIdHashArray = NULL;
     OvsFreeMemory(switchContext->portNoHashArray);
+    switchContext->portNoHashArray = NULL;
     OvsFreeMemory(switchContext->pidHashArray);
+    switchContext->pidHashArray = NULL;
     OvsDeleteFlowTable(&switchContext->datapath);
     OvsCleanupBufferPool(switchContext);
     OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext);
--
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=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=xKHZjHUGhbkwNXBDEoDaCab0EiVqUxWjgM2k%2BDSkvXI%3D%0A&s=d67161495e9947d4a44ddb208da0b8d6fcd7edfc48e42e29f0616f393dad8456