datapath-windows: Fix bugs in cleaner threads

Conntrack, Conntrack-related, Stt, and IP fragmentation
have cleaner threads that run periodically to clean
up their respective tables. During driver unload,
OvsExtDetach() calls into routines that are meant
for explicitly cleaning these tables up and freeing
the resources associated with these threads.

If during driver unload, these cleaner threads run
immediately after the resources are freed, such as locks
used by these threads, then the cleaner threads result
in a kernel crash since they try to acquire locks
that have already been freed.

For eg, OvsIpFragmentEntryCleaner() caused a kernel
crash because it tried to acquire a lock that was
already freed by OvsCleanupIpFragment().

The fix is to simply exit the cleaner thread if the
lock associated with the thread is not initialized,
because the only way the threads can run when the lock
is invalid is when the lock has been freed up during
driver unload.

Testing done:
Verified that cleaner threads run as expected without
crashing during driver unload.

Signed-off-by: Shashank Ram <rams@vmware.com>
Acked-by: Anand Kumar <kumaranand@vmware.com>
Signed-off-by: Gurucharan Shetty <guru@ovn.org>

4 files changed, 19 insertions, 3 deletions

diff --git a/datapath-windows/ovsext/Conntrack-related.c b/datapath-windows/ovsext/Conntrack-related.c
index 16ed6f760..ec4b5366f 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -181,10 +181,14 @@ OvsCtRelatedEntryCleaner(PVOID data)
     POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data;
     PLIST_ENTRY link, next;
     POVS_CT_REL_ENTRY entry;
+    LOCK_STATE_EX lockState;
     BOOLEAN success = TRUE;
 
     while (success) {
-        LOCK_STATE_EX lockState;
+        if (ovsCtRelatedLockObj == NULL) {
+            /* Lock has been freed by 'OvsCleanupCtRelated()' */
+            break;
+        }
         NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
         if (context->exit) {
             NdisReleaseRWLock(ovsCtRelatedLockObj, &lockState);
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index f0d135be3..3203411a8 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -980,10 +980,14 @@ OvsConntrackEntryCleaner(PVOID data)
     POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data;
     PLIST_ENTRY link, next;
     POVS_CT_ENTRY entry;
+    LOCK_STATE_EX lockState;
     BOOLEAN success = TRUE;
 
     while (success) {
-        LOCK_STATE_EX lockState;
+        if (ovsConntrackLockObj == NULL) {
+            /* Lock has been freed by 'OvsCleanupConntrack()' */
+            break;
+        }
         NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0);
         if (context->exit) {
             NdisReleaseRWLock(ovsConntrackLockObj, &lockState);
diff --git a/datapath-windows/ovsext/IpFragment.c b/datapath-windows/ovsext/IpFragment.c
index fee361a04..ad48834a6 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -444,10 +444,14 @@ OvsIpFragmentEntryCleaner(PVOID data)
     POVS_IPFRAG_THREAD_CTX context = (POVS_IPFRAG_THREAD_CTX)data;
     PLIST_ENTRY link, next;
     POVS_IPFRAG_ENTRY entry;
+    LOCK_STATE_EX lockState;
     BOOLEAN success = TRUE;
 
     while (success) {
-        LOCK_STATE_EX lockState;
+        if (ovsIpFragmentHashLockObj == NULL) {
+            /* Lock has been freed by 'OvsCleanupIpFragment()' */
+            break;
+        }
         NdisAcquireRWLockWrite(ovsIpFragmentHashLockObj, &lockState, 0);
         if (context->exit) {
             NdisReleaseRWLock(ovsIpFragmentHashLockObj, &lockState);
diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index b6236fd7f..f98070f33 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -551,6 +551,10 @@ OvsSttDefragCleaner(PVOID data)
     BOOLEAN success = TRUE;
 
     while (success) {
+        if (&OvsSttSpinLock == NULL) {
+            /* Lock has been freed by 'OvsCleanupSttDefragmentation()' */
+            break;
+        }
         NdisAcquireSpinLock(&OvsSttSpinLock);
         if (context->exit) {
             NdisReleaseSpinLock(&OvsSttSpinLock);