Closed Bug 1711553 Opened 3 years ago Closed 3 years ago

Invalid Win32k use in content process [xul!create_dwrite_rendering_params]

Categories

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

All
Windows
defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox93 --- fixed

People

(Reporter: cmartin, Assigned: lsalzman)

References

(Regressed 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Call stack:

00 000000b5`ccff6bf8 00007ffc`33a6725a win32u!NtUserEnumDisplayDevices
01 000000b5`ccff6c00 00007ffc`2510376a USER32!EnumDisplayDevicesW+0x6a
02 000000b5`ccff6fd0 00007ffc`25103649 dwrite!DWriteRenderingParams::CreateRenderingParamsFromDeviceName+0xf6
03 000000b5`ccff7430 00007ffc`25103ab7 dwrite!DWriteRenderingParams::Create+0x19
04 000000b5`ccff7460 00007ffb`c6567b20 dwrite!DWriteFactory::CreateRenderingParamsInternal+0x47
05 (Inline Function) --------`-------- xul!create_dwrite_rendering_params+0x24 [c:\moz\mozilla-central\gfx\skia\skia\src\utils\win\SkDWrite.cpp @ 65] 
06 (Inline Function) --------`-------- xul!SkOnce::operator()+0x3c [c:\moz\mozilla-central\gfx\skia\skia\include\private\SkOnce.h @ 36] 
07 000000b5`ccff74d0 00007ffb`c65243d5 xul!sk_get_dwrite_default_rendering_params+0x40 [c:\moz\mozilla-central\gfx\skia\skia\src\utils\win\SkDWrite.cpp @ 72] 
08 000000b5`ccff7500 00007ffb`c65273bc xul!SkScalerContext_DW::SkScalerContext_DW+0x875 [c:\moz\mozilla-central\gfx\skia\skia\src\ports\SkScalerContext_win_dw.cpp @ 350] 
09 000000b5`ccff7610 00007ffb`c65af21e xul!DWriteFontTypeface::onCreateScalerContext+0x4c [c:\moz\mozilla-central\gfx\skia\skia\src\ports\SkTypeface_win_dw.cpp @ 360] 
0a 000000b5`ccff7670 00007ffb`c65d3df8 xul!SkTypeface::createScalerContext+0x2e [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkScalerContext.cpp @ 904] 
0b 000000b5`ccff76d0 00007ffb`c65d3c59 xul!SkTypeface::onComputeBounds+0x168 [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTypeface.cpp @ 406] 
0c (Inline Function) --------`-------- xul!SkTypeface::getBounds::<unnamed-tag>::operator()+0x1a [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTypeface.cpp @ 378] 
0d (Inline Function) --------`-------- xul!SkOnce::operator()+0x2c [c:\moz\mozilla-central\gfx\skia\skia\include\private\SkOnce.h @ 36] 
0e 000000b5`ccff78a0 00007ffb`c65786a7 xul!SkTypeface::getBounds+0x39 [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTypeface.cpp @ 377] 
0f 000000b5`ccff78e0 00007ffb`c65d283a xul!SkFontPriv::GetFontBounds+0x87 [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkFont.cpp @ 400] 
10 000000b5`ccff7960 00007ffb`c65d2413 xul!SkTextBlobBuilder::ConservativeRunBounds+0x3a [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTextBlob.cpp @ 308] 
11 (Inline Function) --------`-------- xul!SkTextBlobBuilder::updateDeferredBounds+0x25 [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTextBlob.cpp @ 373] 
12 000000b5`ccff7a40 00007ffb`c5c9b826 xul!SkTextBlobBuilder::make+0x43 [c:\moz\mozilla-central\gfx\skia\skia\src\core\SkTextBlob.cpp @ 605] 
13 (Inline Function) --------`-------- xul!CreateTextBlob+0x4a3 [c:\moz\mozilla-central\layout\painting\nsCSSRendering.cpp @ 3863] 
14 000000b5`ccff7a90 00007ffb`c5b54064 xul!nsCSSRendering::PaintDecorationLine+0xab6 [c:\moz\mozilla-central\layout\painting\nsCSSRendering.cpp @ 4069] 
15 000000b5`ccff8570 00007ffb`c5b59db9 xul!nsTextFrame::PaintDecorationLine+0xf4 [c:\moz\mozilla-central\layout\generic\nsTextFrame.cpp @ 5705] 
16 000000b5`ccff8630 00007ffb`c5b5985a xul!nsTextFrame::DrawTextRunAndDecorations::<unnamed-tag>::operator()+0x299 [c:\moz\mozilla-central\layout\generic\nsTextFrame.cpp @ 7061] 
17 000000b5`ccff8750 00007ffb`c5b5563c xul!nsTextFrame::DrawTextRunAndDecorations+0x81a [c:\moz\mozilla-central\layout\generic\nsTextFrame.cpp @ 7102] 
18 000000b5`ccff8a00 00007ffb`c5b58a10 xul!nsTextFrame::DrawText+0x12c [c:\moz\mozilla-central\layout\generic\nsTextFrame.cpp @ 7168] 
19 000000b5`ccff8b40 00007ffb`c5ccab9a xul!nsTextFrame::PaintText+0x880 [c:\moz\mozilla-central\layout\generic\nsTextFrame.cpp @ 6855] 
1a 000000b5`ccff8e70 00007ffb`c5ccaebf xul!nsDisplayText::RenderToContext+0x34a [c:\moz\mozilla-central\layout\painting\nsDisplayList.cpp @ 8972] 
1b 000000b5`ccff9000 00007ffb`c3da6867 xul!nsDisplayText::CreateWebRenderCommands+0x2af [c:\moz\mozilla-central\layout\painting\nsDisplayList.cpp @ 8896] 
1c 000000b5`ccff90e0 00007ffb`c3da5b6f xul!mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommands+0xa7 [c:\moz\mozilla-central\gfx\layers\wr\WebRenderCommandBuilder.cpp @ 1676] 
1d 000000b5`ccff9150 00007ffb`c5ccd648 xul!mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList+0x6ef [c:\moz\mozilla-central\gfx\layers\wr\WebRenderCommandBuilder.cpp @ 1796] 
Assignee: nobody → cmartin
Status: NEW → ASSIGNED
Severity: -- → S4
Priority: -- → P2

It looks like this is an addition to Skia that we added. Lee likely has opinions on the best way to avoid it

Flags: needinfo?(lsalzman)

I don't see any particularly great way to avoid this, as we are using it to determine the rendering parameters and modes for the font. As well, this would precede a bunch of other DWrite calls...

Flags: needinfo?(lsalzman)

After discussion with jfkthame, it seems like bobowen and bug 1698946 will lay the groundwork for what needs to be done here. After that lands, we will need to pass those rendering parameters into Skia, which will bypass the need for it to query them all over again in the content process. It's probably best if I do that when we're ready.

Assignee: cmartin → lsalzman
Depends on: 1698946

(In reply to Lee Salzman [:lsalzman] from comment #3)

After discussion with jfkthame, it seems like bobowen and bug 1698946 will lay the groundwork for what needs to be done here. After that lands, we will need to pass those rendering parameters into Skia, which will bypass the need for it to query them all over again in the content process. It's probably best if I do that when we're ready.

The main point of bug 1698946 is to retrieve the various parameters in the parent process and then make them available to the other processes without the win32k API usage.
This includes making the creation of IDWriteRenderingParams lazy, so that we don't try and create them in the content process now that we no longer (hopefully) need them for DrawTargetD2D1::FillGlyphs.

Looks like we could replace the use of GetRecommendedRenderingMode and pass in our own setting or something and remove the need for the IDWriteRenderingParams and sk_get_dwrite_default_rendering_params.
I also wonder if we'll hit other APIs that also use win32k later on.
Perhaps some of the stacks in bug 1383522 that aren't this one, although they are a bit short to be able to see exactly what's causing them.

See Also: → 1383522

Instead of letting Skia query the default rendering parameters from the OS, instead pass
through parameters constructed from DWriteSettings that Skia can use.

This attempts to emulate the behavior of GetRecommendedRenderingMode without
actually calling it. In addition, it allows some Gecko-specific behavior
like overriding the render mode explicitly that allows some simplification
of the decision-making.

Inside GetRecommendedRenderingMode, natural is only allowed if <= 20 size.
This allows us to thus decide mostly based on whether the size is > 20 or
if an explicit mode was specified. In the remaining case where we need
to check a GASP table if available, we defer to the symmetric flag. If
there is no GASP, we assume natural.

Attachment #9234964 - Attachment is obsolete: true
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/44306972349f Avoid use of GetRecommendedRenderingMode in Skia. r=jrmuizel
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch
Regressions: 1734853
Regressions: 1737777
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: