[PATCH 0/1] MR7671: wintrust: Don't set desktop window when initializing provider data.

[PATCH 1/1] wintrust: Don't set desktop window when initializing provider data.

[Paul Gofman]

From: Paul Gofman

---
 dlls/wintrust/tests/softpub.c | 21 +++++++++++++++++++++
 dlls/wintrust/wintrust_main.c | 10 ++--------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/dlls/wintrust/tests/softpub.c b/dlls/wintrust/tests/softpub.c
index 9654c296a78..7091f5a7553 100644
--- a/dlls/wintrust/tests/softpub.c
+++ b/dlls/wintrust/tests/softpub.c
@@ -832,6 +832,7 @@ static void test_wintrust(void)
     LONG r;
     HRESULT hr;
     WCHAR pathW[MAX_PATH];
+    CRYPT_PROVIDER_DATA *prov_data;

memset(&wtd, 0, sizeof(wtd));
     wtd.cbStruct = sizeof(wtd);
@@ -867,10 +868,30 @@ static void test_wintrust(void)
     ok(r == S_OK, "WinVerifyTrust failed: %08lx\n", r);
     wtd.dwStateAction = WTD_STATEACTION_VERIFY;
     SetLastError(0xdeadbeef);
+    wtd.hWVTStateData = NULL;
     hr = WinVerifyTrustEx(INVALID_HANDLE_VALUE, &generic_action_v2, &wtd);
     ok(hr == GetLastError(), "expected %08lx, got %08lx\n", GetLastError(), hr);
     ok(hr == TRUST_E_NOSIGNATURE || hr == CRYPT_E_FILE_ERROR,
      "expected TRUST_E_NOSIGNATURE or CRYPT_E_FILE_ERROR, got %08lx\n", hr);
+    prov_data = WTHelperProvDataFromStateData(wtd.hWVTStateData);
+    ok(!!prov_data, "got NULL.\n");
+    ok(prov_data->hWndParent == INVALID_HANDLE_VALUE, "got %p.\n", prov_data->hWndParent);
+    wtd.dwStateAction = WTD_STATEACTION_CLOSE;
+    SetLastError(0xdeadbeef);
+    r = WinVerifyTrust(INVALID_HANDLE_VALUE, &generic_action_v2, &wtd);
+    ok(GetLastError() == 0xdeadbeef, "expected 0xdeadbeef, got %08lx\n", GetLastError());
+    ok(r == S_OK, "WinVerifyTrust failed: %08lx\n", r);
+
+    wtd.dwStateAction = WTD_STATEACTION_VERIFY;
+    SetLastError(0xdeadbeef);
+    wtd.hWVTStateData = NULL;
+    hr = WinVerifyTrustEx(NULL, &generic_action_v2, &wtd);
+    ok(hr == GetLastError(), "expected %08lx, got %08lx\n", GetLastError(), hr);
+    ok(hr == TRUST_E_NOSIGNATURE || hr == CRYPT_E_FILE_ERROR,
+     "expected TRUST_E_NOSIGNATURE or CRYPT_E_FILE_ERROR, got %08lx\n", hr);
+    prov_data = WTHelperProvDataFromStateData(wtd.hWVTStateData);
+    ok(!!prov_data, "got NULL.\n");
+    ok(!prov_data->hWndParent, "got %p.\n", prov_data->hWndParent);
     wtd.dwStateAction = WTD_STATEACTION_CLOSE;
     SetLastError(0xdeadbeef);
     r = WinVerifyTrust(INVALID_HANDLE_VALUE, &generic_action_v2, &wtd);
diff --git a/dlls/wintrust/wintrust_main.c b/dlls/wintrust/wintrust_main.c
index 29f2f9c7a4f..66e274a24ae 100644
--- a/dlls/wintrust/wintrust_main.c
+++ b/dlls/wintrust/wintrust_main.c
@@ -277,10 +277,7 @@ static LONG WINTRUST_DefaultVerify(HWND hwnd, GUID *actionID,
     if (WVT_ISINSTRUCT(WINTRUST_DATA, data->cbStruct, pSignatureSettings))
         provData->pSigSettings = data->pSignatureSettings;

-    if (hwnd == INVALID_HANDLE_VALUE)
-        provData->hWndParent = GetDesktopWindow();
-    else
-        provData->hWndParent = hwnd;
+    provData->hWndParent = hwnd;
     provData->pgActionID = actionID;
     WintrustGetRegPolicyFlags(&provData->dwRegPolicySettings);

@@ -468,10 +465,7 @@ static LONG WINTRUST_CertVerify(HWND hwnd, GUID *actionID,

data->hWVTStateData = provData;
     provData->pWintrustData = data;
-    if (hwnd == INVALID_HANDLE_VALUE)
-        provData->hWndParent = GetDesktopWindow();
-    else
-        provData->hWndParent = hwnd;
+    provData->hWndParent = hwnd;
     provData->pgActionID = actionID;
     WintrustGetRegPolicyFlags(&provData->dwRegPolicySettings);

[Paul Gofman (＠gofman)]

My motivation for this patch is a bit weird, it (hopefully) fixes random lockup on GTAV Enhanced start (from Rockstar Launcher). The actual deadlock happens between one thread loading and initializing winex11.drv and another thread which called WinVerifyTrust which called GetDesktopWindow and triggered explorer / desktop initialization, that locks up between loader lock and some of user32 or winex11 locks.

Of course the core reason for lockup is not in wintrust. But wintrust has nothing to do with getting desktop window and thus pulling / initializing the whole win32 subsystem when not asked to. So it is probably beneficial not to pull all that regardless of user32 / loader issues. Besides the added test, I checked that we don't use hWndParent from provider data anywhere (and if we did handling should've probably be done in provider, also INVALID_HANDLE_VALUE likely means that no user interaction is supposed to take place and likely only NULL handle may imply using desktop window for anything).

[PATCH 0/1] MR7671: wintrust: Don't set desktop window when initializing provider data. - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.