[PATCH] wintrust: Cache provider functions in WintrustLoadFunctionPointers().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
    As comment in WINTRUST_ReadProviderFromReg() says, there is no way to free a provider
    and module references are just leaked. This can potentially overflow module
    LoadCount (which is currently 16 bit in Wine) if the app calls WintrustLoadFunctionPointers()
    a lot. Then there is a specific issue with DeathLoop where every LoadLibrary call
    triggers wintrust.WintrustLoadFunctionPointers() call (the process is triggered from LDR notification
    callback while the actual call is from another thread). The latter either deadlocks
    or (with WIP relaxed loader locking patches) triggers an infinite loop and eventually crashes.

Caching the providers data solves both problems.

dlls/wintrust/register.c | 60 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/dlls/wintrust/register.c b/dlls/wintrust/register.c
index effe9d18f20..b15fbe5ab35 100644
--- a/dlls/wintrust/register.c
+++ b/dlls/wintrust/register.c
@@ -34,6 +34,7 @@
 #include "mssip.h"
 #include "wintrust_priv.h"
 #include "wine/debug.h"
+#include "wine/heap.h"

WINE_DEFAULT_DEBUG_CHANNEL(wintrust);

@@ -835,6 +836,23 @@ error_close_key:
     return Func;
 }

+static CRITICAL_SECTION cache_cs;
+static CRITICAL_SECTION_DEBUG cache_cs_debug =
+{
+    0, 0, &cache_cs,
+    { &cache_cs_debug.ProcessLocksList, &cache_cs_debug.ProcessLocksList },
+      0, 0, { (DWORD_PTR)(__FILE__ ": cache_cs") }
+};
+static CRITICAL_SECTION cache_cs = { &cache_cs_debug, -1, 0, 0, 0, 0 };
+
+static struct provider_cache_entry
+{
+    GUID guid;
+    CRYPT_PROVIDER_FUNCTIONS provider_functions;
+}
+*provider_cache;
+static unsigned int provider_cache_size;
+
 /***********************************************************************
  *              WintrustLoadFunctionPointers (WINTRUST.@)
  */
@@ -842,6 +860,8 @@ BOOL WINAPI WintrustLoadFunctionPointers( GUID* pgActionID,
                                           CRYPT_PROVIDER_FUNCTIONS* pPfns )
 {
     WCHAR GuidString[39];
+    BOOL cached = FALSE;
+    unsigned int i;

TRACE("(%s %p)\n", debugstr_guid(pgActionID), pPfns);

@@ -853,6 +873,20 @@ BOOL WINAPI WintrustLoadFunctionPointers( GUID* pgActionID,
     }
     if (pPfns->cbStruct != sizeof(CRYPT_PROVIDER_FUNCTIONS)) return FALSE;

+    EnterCriticalSection( &cache_cs );
+    for (i = 0; i < provider_cache_size; ++i)
+    {
+        if (IsEqualGUID( &provider_cache[i].guid, pgActionID ))
+        {
+            TRACE( "Using cached data.\n" );
+            *pPfns = provider_cache[i].provider_functions;
+            cached = TRUE;
+            break;
+        }
+    }
+    LeaveCriticalSection( &cache_cs );
+    if (cached) return TRUE;
+
     /* Create this string only once, instead of in the helper function */
     WINTRUST_Guid2Wstr( pgActionID, GuidString);

@@ -873,6 +907,32 @@ BOOL WINAPI WintrustLoadFunctionPointers( GUID* pgActionID,
     pPfns->pfnTestFinalPolicy = (PFN_PROVIDER_TESTFINALPOLICY_CALL)WINTRUST_ReadProviderFromReg(GuidString, DiagnosticPolicy);
     pPfns->pfnCleanupPolicy = (PFN_PROVIDER_CLEANUP_CALL)WINTRUST_ReadProviderFromReg(GuidString, Cleanup);

+    EnterCriticalSection( &cache_cs );
+    for (i = 0; i < provider_cache_size; ++i)
+        if (IsEqualGUID( &provider_cache[i].guid, pgActionID )) break;
+
+    if (i == provider_cache_size)
+    {
+        struct provider_cache_entry *new;
+
+        new = heap_realloc( provider_cache, (provider_cache_size + 1) * sizeof(*new) );
+        if (new)
+        {
+            provider_cache = new;
+            ++provider_cache_size;
+        }
+        else
+        {
+            ERR( "No memory.\n" );
+        }
+    }
+    if (i < provider_cache_size)
+    {
+        provider_cache[i].guid = *pgActionID;
+        provider_cache[i].provider_functions = *pPfns;
+    }
+    LeaveCriticalSection( &cache_cs );
+
     return TRUE;
 }