krb5 commit: Fix KfW thread-local storage allocation issues

[Benjamin Kaduk]

https://github.com/krb5/krb5/commit/a4418f619be053c7429e307f78d9694b2f798c65
commit a4418f619be053c7429e307f78d9694b2f798c65
Author: Kevin Wasserman <kevin.wasserman at painless-security.com>
Date:   Tue Aug 21 11:44:46 2012 -0400

    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 at painless-security.com>
    
    ticket: 7340 (new)
    target_version: 1.10.4
    tags: pullup

 src/ccapi/common/win/tls.c    |    8 +++++++-
 src/ccapi/lib/win/dllmain.cxx |   27 ++++++++++++++++-----------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/ccapi/common/win/tls.c b/src/ccapi/common/win/tls.c
index 48a5bd5..5743ddb 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 f9d1e2a..82cacad 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!