Fix KfW thread-local storage allocation issues

Allocate thread-local storage on demand; don't rely on
the DLL_THREAD_ATTACH case in DllMain() since pre-existing
threads will never execute that code.

Signed-off-by: Kevin Wasserman <kevin.wasserman@painless-security.com>

ticket: 7340 (new)
target_version: 1.10.4
tags: pullup

2 files changed, 23 insertions, 12 deletions

diff --git a/src/ccapi/common/win/tls.c b/src/ccapi/common/win/tls.c
index 48a5bd5a45..5743ddb203 100644
--- a/src/ccapi/common/win/tls.c
+++ b/src/ccapi/common/win/tls.c
@@ -100,7 +100,13 @@ BOOL WINAPI GetTspData(DWORD dwTlsIndex, struct tspdata**  pdw) {
     struct tspdata*  pData;      // The stored memory pointer
 
     pData = (struct tspdata*)TlsGetValue(dwTlsIndex);
-    if (pData == NULL) return FALSE;
+    if (pData == NULL) {
+        pData = malloc(sizeof(*pData));
+        if (pData == NULL)
+            return FALSE;
+        memset(pData, 0, sizeof(*pData));
+        TlsSetValue(dwTlsIndex, pData);
+    }
     (*pdw) = pData;
     return TRUE;
     }
diff --git a/src/ccapi/lib/win/dllmain.cxx b/src/ccapi/lib/win/dllmain.cxx
index f9d1e2a437..82cacad9c8 100644
--- a/src/ccapi/lib/win/dllmain.cxx
+++ b/src/ccapi/lib/win/dllmain.cxx
@@ -98,13 +98,13 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL,     // DLL module handle
  
         // The attached process creates a new thread:
         case DLL_THREAD_ATTACH:
-            // Initialize the TLS index for this thread:
-            ptspdata    = (struct tspdata*) LocalAlloc(LPTR, sizeof(struct tspdata)); 
-            cci_debug_printf("%s DLL_THREAD_ATTACH; tsp*:0x%X", __FUNCTION__, ptspdata);
-            if (ptspdata == NULL) return FALSE;
-            fIgnore     = TlsSetValue(dwTlsIndex, ptspdata); 
-
-            memset(ptspdata, 0, sizeof(struct tspdata));
+            cci_debug_printf("%s DLL_THREAD_ATTACH", __FUNCTION__);
+            // Don't actually rely on this case for allocation of resources.
+            // Applications (like SecureCRT) may have threads already
+            // created (say 'A' and 'B') before the dll is loaded. If the dll
+            // is loaded in thread 'A' but then used in thread 'B', thread 'B'
+            // will never execute this code.
+            fIgnore     = TlsSetValue(dwTlsIndex, NULL);
 
             // Do not call cci_ipc_thread_init() yet; defer until we actually
             // need it.  On XP, cci_ipc_thread_init() will cause additional
@@ -116,10 +116,10 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL,     // DLL module handle
         // The thread of the attached process terminates:
         case DLL_THREAD_DETACH: 
             cci_debug_printf("%s DLL_THREAD_DETACH", __FUNCTION__);
-            // Release the allocated memory for this thread:
+            // Release the allocated memory for this thread
             ptspdata = (struct tspdata*)TlsGetValue(dwTlsIndex); 
             if (ptspdata != NULL) {
-                LocalFree((HLOCAL) ptspdata); 
+                free(ptspdata);
                 TlsSetValue(dwTlsIndex, NULL); 
                 }
             break; 
@@ -183,11 +183,16 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL,     // DLL module handle
 
             // Release the allocated memory for this thread:
             ptspdata = (struct tspdata*)TlsGetValue(dwTlsIndex); 
-            if (ptspdata != NULL) LocalFree((HLOCAL) ptspdata);
+            if (ptspdata != NULL)
+                free(ptspdata);
             TlsFree(dwTlsIndex);    // Release the TLS index.
+            // Ideally, we would enumerate all other threads here and
+            // release their thread local storage as well.
             break; 
  
-        default: break; 
+        default:
+            cci_debug_printf("%s unexpected reason %d", __FUNCTION__, fdwReason);
+            break;
         } 
  
     UNREFERENCED_PARAMETER(hinstDLL);       // no whining!