Closed Bug 1546154 Opened 6 years ago Closed 4 years ago

Fix xul.dll dependencies to not load user32 and gdi32 when running in a sandboxed child process with win32k lockdown

Categories

(Toolkit :: General, task, P3)

Unspecified
Windows
task

Tracking

()

RESOLVED FIXED
94 Branch
Tracking Status
firefox94 --- fixed

People

(Reporter: bugzilla, Assigned: bobowen)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

Attachments

(8 files, 1 obsolete file)

7.89 KB, patch
Details | Diff | Splinter Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

(Filing this under toolkit since the moz.build that governs xul.dll's linking is under toolkit/library.)

As we learned in bug 1535704, system DLLs may make assumptions about the availability of win32k by checking for the presence of user32, without checking the state of the win32k lockdown process mitigation policy itself.

In order to avoid future occurrences of this, and also to improve memory usage, we should reorganize xul.dll's dependencies so that these libraries are not loaded when they are unnecessary.

Note that there may be transitive dependencies due to other libraries that are imported by xul.

Attachment #9209744 - Attachment description: Prevent load of user32 and gdi32 → WIP - Prevent load of user32 and gdi32
Attachment #9209744 - Attachment is patch: true

I took Bob's patch from Comment 2 and broke it up over Bug 1701757 and Bug 1701778. They should be pretty straightforward to land :)

Here's my latest WIP that I need to break up.
It would be interesting if this fixes what you're seeing.

Assignee: nobody → bobowencode
Attachment #9209744 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(cmartin)

Your patch got rid of quite a few DLLs that were loading ole32.dll. With a few more functions patched to api-ms-win-core-com-l1-1-0.dll I'm able to start to see content stuff pop up without an ole32.dll load, so that's awesome.

I've switched to another task for now though (investigating the failures from that Try build that I made with the Faux Lockdown), since I suspect you are also currently patching out a bunch of these calls and I don't want to redo the same work :)

Flags: needinfo?(cmartin)

This is required because ole32.dll is now delay loaded by xul.dll.

Depends on D124931

Threads are implicitly members of the multi-threaded apartment and calls to
CoInitializeEx (and CoUninitializeEx) cause user32 to load.

Depends on D124932

Threads are implicitly members of the multi-threaded apartment and calls to
CoInitializeEx (and CoUninitializeEx) cause user32 to load.

Depends on D124933

Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/autoland/rev/bbb0b25450f5 p1: Call CommandLineToArgvW via API set when possible to prevent shell32 loading. r=handyman https://hg.mozilla.org/integration/autoland/rev/7f50fca0c2cd p2: Delay load DLLs in xul to prevent user32 from automatically loading. r=glandium https://hg.mozilla.org/integration/autoland/rev/f3e005f5fedc p3: Explicitly load COM functions from combase.dll to prevent ole32 loading. r=Jamie https://hg.mozilla.org/integration/autoland/rev/cfda47fb0a46 p4: pre-load ole32.dll for GMPs that require it for OPM. r=bryce https://hg.mozilla.org/integration/autoland/rev/cef0aa18a3ab p5: Remove CoInitializeEx call from TaskController. r=bas https://hg.mozilla.org/integration/autoland/rev/611812ee62a2 p6: Remove MSCOMInitThreadPoolListener. r=padenot https://hg.mozilla.org/integration/autoland/rev/0bd777eee249 p7: Don't use the content temp dir when win32k is locked down. r=handyman

Backed out 7 changesets (Bug 1546154) for causing build bustages on ContentProcess.cpp.
Backout link
Push with failures
Multiple failures: R1, bc2
Failure Log

Flags: needinfo?(bobowencode)
Flags: needinfo?(bobowencode)
Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5b7e29767850 p1: Call CommandLineToArgvW via API set when possible to prevent shell32 loading. r=handyman https://hg.mozilla.org/integration/autoland/rev/e3b1d57b425d p2: Delay load DLLs in xul to prevent user32 from automatically loading. r=glandium https://hg.mozilla.org/integration/autoland/rev/e9ac3dc0abbc p3: Explicitly load COM functions from combase.dll to prevent ole32 loading. r=Jamie https://hg.mozilla.org/integration/autoland/rev/216cf15939f4 p4: pre-load ole32.dll for GMPs that require it for OPM. r=bryce https://hg.mozilla.org/integration/autoland/rev/1595ad239e4e p5: Remove CoInitializeEx call from TaskController. r=bas https://hg.mozilla.org/integration/autoland/rev/11d856647620 p6: Remove MSCOMInitThreadPoolListener. r=padenot https://hg.mozilla.org/integration/autoland/rev/66b784a0294e p7: Don't use the content temp dir when win32k is locked down. r=handyman
Regressions: 1732421
See Also: → 1743427
Depends on: 1755470
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: