[PATCH] wintrust: Cache provider functions in WintrustLoadFunctionPointers().
Paul Gofman
pgofman at codeweavers.com
Mon Oct 4 13:44:32 CDT 2021
Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
---
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;
}
--
2.31.1
More information about the wine-devel
mailing list