Closed Bug 1701770 Opened 3 years ago Closed 3 years ago

Invalid Win32k use in content process [USER32!SetProcessDpiAwarenessContext]

Categories

(Core :: Security: Process Sandboxing, defect, P2)

Product:
Core
Component:
Security: Process Sandboxing
Platform:
All
Windows
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
91 Branch
Tracking Flags:
Tracking Status
firefox91 --- verified
firefox96 --- verified

People

(Reporter: cmartin, Assigned: cmartin)

References

Details

Attachments

(3 files)

Bug 1701770 - Defer Windows DPI Awareness from load time to run time
48 bytes, text/x-phabricator-request
Details | Review
Bug 1701770 - Don't load DPI functions in Win32k Lockdown tab
48 bytes, text/x-phabricator-request
Details | Review
SetProcessDpiAwarenessContext failed
312.68 KB, image/png
Details

Seems to happen during xul.dll loading:

win32u!NtUserSetProcessDpiAwarenessContext
USER32!SetProcessDpiAwarenessContext+0x3d
USER32!InitProcessDpiAwareness+0xd7
USER32!_UserClientDllInitialize+0x535
ntdll!LdrpCallInitRoutine+0x61
ntdll!LdrpInitializeNode+0x1d3
ntdll!LdrpInitializeGraphRecurse+0x42
ntdll!LdrpInitializeGraphRecurse+0xc8
ntdll!LdrpPrepareModuleForExecution+0xbf
ntdll!LdrpLoadDllInternal+0x19a
ntdll!LdrpLoadDll+0xa8
ntdll!LdrLoadDll+0xe4
firefox!mozilla::interceptor::FuncHookCrossProcess<mozilla::interceptor::WindowsDllInterceptor<mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyOutOfProcess> >,long (*)(wchar_t *, unsigned long *, _UNICODE_STRING *, void **)>::operator()+0x19 [c:\moz\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsWindowsDllInterceptor.h @ 254]
firefox!mozilla::freestanding::patched_LdrLoadDll+0x50 [c:\moz\mozilla-central\browser\app\winlauncher\freestanding\DllBlocklist.cpp @ 356]
KERNELBASE!LoadLibraryExW+0x162
firefox!GetLibHandle+0x11 [c:\moz\mozilla-central\xpcom\glue\standalone\nsXPCOMGlue.cpp @ 49]
firefox!ReadDependentCB+0x1b [c:\moz\mozilla-central\xpcom\glue\standalone\nsXPCOMGlue.cpp @ 148]
firefox!ReadDependentCB+0x48 [c:\moz\mozilla-central\xpcom\glue\standalone\nsXPCOMGlue.cpp @ 160]
firefox!XPCOMGlueLoad+0x32a [c:\moz\mozilla-central\xpcom\glue\standalone\nsXPCOMGlue.cpp @ 326]
firefox!mozilla::GetBootstrap+0x3ad [c:\moz\mozilla-central\xpcom\glue\standalone\nsXPCOMGlue.cpp @ 409]
firefox!InitXPCOMGlue+0xd6 [c:\moz\mozilla-central\browser\app\nsBrowserApp.cpp @ 236]
firefox!NS_internal_main+0x27b [c:\moz\mozilla-central\browser\app\nsBrowserApp.cpp @ 305]
firefox!wmain+0x1fe [c:\moz\mozilla-central\toolkit\xre\nsWindowsWMain.cpp @ 131]
firefox!invoke_main+0x22 [d:\agent_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 90]
firefox!__scrt_common_main_seh+0x10c [d:\agent_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 288]
KERNEL32!BaseThreadInitThunk+0x14
ntdll!RtlUserThreadStart+0x21

Assignee: nobody → cmartin
Blocks: 1546154
Status: NEW → ASSIGNED

xul.dll implicitly links to user32.dll. It should be delay-loading.

(In reply to Masatoshi Kimura [:emk] from comment #1)

xul.dll implicitly links to user32.dll. It should be delay-loading.

Yeah, that's bug 1546154.
Unfortunately, there are a whole host of other DLLs that also lead to user32/gdi32 loading (I think I've got most, if not all, of them in the WIP patch on that bug), so you also have to make them delay load and stop them from loading when not really required.

In theory we should be able to get win32k lockdown enabled and still let user32/gdi32 load (that's what chromium did originally).
Hopefully, by the time we get win32k lockdown enabled, we won't be far from stopping all of these things loading, which will be great.

Some further details while I remember (and If I remember correctly), this is because user32.dll is what pulls in information from the manifest to set the DPI awareness.
Chromium doesn't have it in the manifest and uses the function calls directly (when win32k is not locked down), I guess they did this before they managed to stop user32 from loading at all.
For the moment we can probably just let the calls fail.

Chromium also has some other fallback logic for not using PROCESS_PER_MONITOR_DPI_AWARE in win8.1, because EnableChildWindowDpiMessage (an undocumented API) is not available.
We should possibly look into that and see if that is a good reason for moving to using the functions rather that the manifest, but I don't think it blocks initial win32 lockdown.

Severity: -- → S4
Priority: -- → P2

Currently, we set DPI awareness in the manifest files for firefox.exe.
Unfortunately, that causes DPI-related Win32k calls when user32.dll
is loaded.

This changes things to wait until we are sure we're not running in a
Win32k Lockdown Content Process before we attempt to initialize DPI
scaling.

Currently, as part of initialization of xul.dll on Windows we dynamically
load some DPI-related functions on Windows 10. Those functions cannot be used
with Win32k lockdown, and thus there is no point to loading them.

Pushed by cmartin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/263591537722
Defer Windows DPI Awareness from load time to run time r=bobowen,aklotz
https://hg.mozilla.org/integration/autoland/rev/a6f46b0a30e9
Don't load DPI functions in Win32k Lockdown tab r=bobowen
Flags: needinfo?(cmartin)
Pushed by cmartin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4af044ed9208
Defer Windows DPI Awareness from load time to run time r=bobowen,aklotz
https://hg.mozilla.org/integration/autoland/rev/2351f158ef88
Don't load DPI functions in Win32k Lockdown tab r=bobowen

https://hg.mozilla.org/mozilla-central/rev/4af044ed9208
https://hg.mozilla.org/mozilla-central/rev/2351f158ef88

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox91: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

Use FF 91.0a1 (2021-06-18) (32 bit) on Windows 10.
SetProcessDpiAwarenessContext failed when calling Firefox by the app with the High DPI scaling override.

Steps to reproduce:
Set FF as Windows default web browser.
Override High DPI scaling on any app (Incompatible with High DPI scaling).
This time I will use the Shutter app. https://www.den4b.com/downloads/shutter
Run Shutter and click Help> Forum (Online).
FF will stop by errors SetProcessDpiAwarenessContext failed.
You can also test SetProcessDpiAwarenessContext failed with other apps Cyberduck.

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/668013f3e2015d408903d31f55d2ffdd08137bfd

Target Milestone: 91 Branch → ---

Chromium does not check errors from DPI Awareness functions:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/glfw/src/src/win32_init.c;l=564-569?q=setprocessdpiawarenesscontext&ss=chromium
We should also ignore errors (or at least ignore ERROR_ACCESS_DENIED).

Pushed by cmartin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a1b33fd91211
Defer Windows DPI Awareness from load time to run time r=bobowen,aklotz
https://hg.mozilla.org/integration/autoland/rev/51700ead8a96
Don't load DPI functions in Win32k Lockdown tab r=bobowen
Flags: needinfo?(cmartin)

https://hg.mozilla.org/mozilla-central/rev/a1b33fd91211
https://hg.mozilla.org/mozilla-central/rev/51700ead8a96

Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
status-firefox91: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Flags: qe-verify+
Blocks: 1701788

I was able to reproduce the issue using the steps and build mentioned on comment 11.
Verified as fixed on Win 10 using 91.0 (20210804193234), 96.0b2(20211207185904) and 97.0a1(20211208214306).

Status: RESOLVED → VERIFIED
status-firefox91: fixed → verified
status-firefox96: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: