1713973 - [Win32k lockdown] NS_GetComplexLineBreaks crash in [@ RtlpWaitOnCriticalSection | RtlpEnterCriticalSectionContended | RtlEnterCriticalSection | ThaiBreak]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Chris Peterson [:cpeterson]]

This crash seems to be Fission related. There had been only about one crash report per release cycle until Beta 90.0b started today. Just today we have already received 13 reports from 5 different installations and they all had Fission enabled.

All of the crashes are on Windows 10.

Crash report: https://crash-stats.mozilla.org/report/index/dcbfef71-4201-430e-b6f4-f62370210601

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 10 frames of crashing thread:

0 ntdll.dll RtlpWaitOnCriticalSection 
1 ntdll.dll RtlpEnterCriticalSectionContended 
2 ntdll.dll RtlEnterCriticalSection 
3 gdi32full.dll long ThaiBreak 
4 gdi32full.dll ScriptBreak 
5 xul.dll NS_GetComplexLineBreaks intl/lwbrk/nsUniscribeBreaker.cpp:69
6 xul.dll mozilla::intl::LineBreaker::GetJISx4051Breaks intl/lwbrk/LineBreaker.cpp:1113
7 xul.dll nsLineBreaker::FlushCurrentWord dom/base/nsLineBreaker.cpp:83
8 xul.dll nsLineBreaker::Reset dom/base/nsLineBreaker.cpp:501
9 xul.dll BuildTextRunsScanner::FlushLineBreaks layout/generic/nsTextFrame.cpp:1679

Here is a similar NS_GetComplexLineBreaks crash:

Crash report: https://crash-stats.mozilla.org/report/index/a32d53ce-5df3-4a16-986c-fba590210601

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 10 frames of crashing thread:

0 ntdll.dll RtlpWaitOnCriticalSection 
1 ntdll.dll RtlpEnterCriticalSectionContended 
2 ntdll.dll RtlEnterCriticalSection 
3 gdi32full.dll int UspInitMemory 
4 gdi32full.dll CUspShapingClient::AllocMem 
5 textshaping.dll textshaping.dll@0x999f 
6 textshaping.dll textshaping.dll@0x15888 
7 gdi32full.dll XLATEOBJ_piVector 
8 ntdll.dll LdrResolveDelayLoadedAPI 
9 textshaping.dll textshaping.dll@0x8919 
10 gdi32full.dll XLATEOBJ_piVector
11 gdi32full.dll ScriptBreak 
12 xul.dll NS_GetComplexLineBreaks intl/lwbrk/nsUniscribeBreaker.cpp:69
13 xul.dll mozilla::intl::LineBreaker::GetJISx4051Breaks intl/lwbrk/LineBreaker.cpp:1113
14 xul.dll nsLineBreaker::FlushCurrentWord dom/base/nsLineBreaker.cpp:83
15 xul.dll nsLineBreaker::Reset dom/base/nsLineBreaker.cpp:501
16 xul.dll BuildTextRunsScanner::FlushLineBreaks layout/generic/nsTextFrame.cpp:1679

Comment 1

[Jonathan Kew [:jfkthame]]

Bob, is this related to win32k lockdown? I see "contentWin32kLockdownState":1 in the crash annotations...

nsUniscribeBreaker.cpp has code here to fall back to a non-Uniscribe code path, but that's Nightly-only.

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Jonathan Kew (:jfkthame) from comment #1)

Bob, is this related to win32k lockdown? I see "contentWin32kLockdownState":1 in the crash annotations...

Sounds like this is a win32k sandboxing issue, not Fission.

Comment 3

[Bob Owen (:bobowen)]

(In reply to Jonathan Kew (:jfkthame) from comment #1)

Bob, is this related to win32k lockdown? I see "contentWin32kLockdownState":1 in the crash annotations...

nsUniscribeBreaker.cpp has code here to fall back to a non-Uniscribe code path, but that's Nightly-only.

Yes, it looks like people must have flipped the content win32k lockdown pref.
The pref has been there for a while, so it's possible that people have tried it before and crashed elsewhere, but now it works to a greater extent, but crashes if they hit this.

Comment 4

[Myron]

I had a problem and it does look like it is related to win32k lockdown. All is working fine now. Everything seemed to, sort of, work until I tried to open a tab in a container, bit it didn't always. Where the latest ones have one thing in common where I where the tab crashed every single time
I tried to access accounts.google.com.

I had enabled security.sandbox.content.win32k-disable. Seen this bug report, remembered I enabled it. Disabled and all is now working. Hope he crash reports are of use to whoever will be looking into this.

I'm on FF 90.0b9.

Related (multiple) crash reports:

bp-cd4f294c-8909-4ee3-857f-de4580210618 	18/06/2021, 12:32 	
bp-9efff08f-1a7c-49cd-a20c-1a8270210618 	18/06/2021, 12:32 	
bp-56ef3d01-c14f-4e76-ba69-545310210618 	18/06/2021, 12:29 	
bp-18bf83c5-26eb-4daf-84f5-6b9d90210618 	18/06/2021, 12:29 	
bp-37a26b19-e0e3-4666-9d93-cd6420210618 	18/06/2021, 12:21 	
bp-d028e681-a3d5-4289-b777-28e630210618 	18/06/2021, 12:19 	
bp-4a3f1d8c-5091-46cd-a4dc-d83420210618 	18/06/2021, 12:19 	
bp-8879d4f2-c0c4-434b-8391-649fd0210618 	18/06/2021, 12:18 	
bp-590f8647-102a-48d3-80fc-7a06d0210618 	18/06/2021, 12:17 	
bp-9f919eae-4e63-4bcf-97d9-b3b7e0210618 	18/06/2021, 12:17 	
bp-31ceba46-c859-464e-83bb-4a4730210617 	17/06/2021, 17:51 	
bp-49a4b65d-1442-4723-8a9d-45e880210617 	17/06/2021, 17:51 	
bp-215c71c8-a945-4b6b-8ed8-9f66c0210617 	17/06/2021, 17:51 	
bp-95dd0bba-3463-4285-99c0-fa1a40210617 	17/06/2021, 17:51 	
bp-96998fed-6c01-42ba-b6d9-2fa590210617 	17/06/2021, 17:51 	
bp-dbbfad14-1370-4f45-b220-ce0870210617 	17/06/2021, 17:51 	
bp-86aecfc0-cea3-411c-9bdf-ddd830210617 	17/06/2021, 17:51 	
bp-b27baac7-01a8-4fe2-a71b-c3db80210617 	17/06/2021, 17:51 	
bp-a99c232f-45ac-49d9-a9fc-d5a5c0210617 	17/06/2021, 17:51 	
bp-5f938042-9799-4b32-9b3a-9a5fb0210617 	17/06/2021, 17:51 	
bp-bce9623c-e8b9-4ec4-8aea-4c3600210617 	17/06/2021, 17:51 	
bp-5730703c-4750-465b-ba68-8f0720210617 	17/06/2021, 17:51 	
bp-7d5cdee9-712a-4318-9512-ff2fd0210617 	17/06/2021, 17:51 	
bp-dc431137-e6e0-4ce4-af16-5fd380210617 	17/06/2021, 17:51 	
bp-8c3533d8-dbc4-4a88-a4f9-7ecde0210617 	17/06/2021, 17:51 	
bp-ff32e4c0-0523-43ab-b3bd-a346d0210617 	17/06/2021, 17:51 	
bp-ed0f07ea-17a8-4584-acc7-618820210617 	17/06/2021, 17:51 	
bp-a13e818c-4275-486c-985e-71cae0210617 	17/06/2021, 17:51 	
bp-ac868e9a-1168-4122-8899-317260210617 	17/06/2021, 17:50 	
bp-6dc31c73-2e7b-4e3c-99cf-5264c0210617 	17/06/2021, 17:50 	
bp-5c1a8c73-33e2-415d-a217-65c570210617 	17/06/2021, 17:50 	
bp-fec59fdb-7954-4dc9-b2b5-e52100210617 	17/06/2021, 17:50 	
bp-ed874129-2246-45ff-a6f5-f7efb0210617 	17/06/2021, 17:50 	
bp-d1849b45-2c30-4626-a815-9484d0210617 	17/06/2021, 17:49 	
bp-00fbe052-732f-40c4-b801-0c7ec0210617 	17/06/2021, 17:48 	
bp-374e1ba1-dde5-40d0-b0ff-3687d0210617 	17/06/2021, 17:48 	
bp-de338bf0-bbeb-442e-8792-8a53f0210617 	17/06/2021, 17:48 	
bp-3552e5d9-e1ec-4d63-aea7-a3a0c0210617 	17/06/2021, 17:47 	
bp-9089670f-32ad-445d-9437-2d1c90210616 	16/06/2021, 17:40 	
bp-362d8f1c-d596-492a-a634-b226a0210616 	16/06/2021, 17:39 	

Comment 5

[Myron]

It was not Fission. Same crashes were occurring when I turned off Fission.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Bob Owen (:bobowen) from comment #3)

Yes, it looks like people must have flipped the content win32k lockdown pref.

So to clarify, this crash doesn't affect the regular user population, right? This only affects folks who have manually toggled this about:config pref?

(Also, does that mean this belongs in a different component, or is it classified correctly?)

Comment 7

[Bob Owen (:bobowen)]

(In reply to Daniel Holbert [:dholbert] from comment #6)

(In reply to Bob Owen (:bobowen) from comment #3)

Yes, it looks like people must have flipped the content win32k lockdown pref.

So to clarify, this crash doesn't affect the regular user population, right? This only affects folks who have manually toggled this about:config pref?

(Also, does that mean this belongs in a different component, or is it classified correctly?)

Yes this is with a pref flip, I guess it could be people using a profile from Nightly where they had used the Nightly Experiments to flip the pref. I'm not sure how easy that is to do any more.

Over the classification, because sandboxing tends to be cross-cutting this is a bit of a grey area.
It would be nice to get to a position where the sandboxing component is only used for bugs in the actual sandboxing code, as opposed to the effects of it. However it does often get used for bugs caused by code that then fails in components elsewhere.
This becomes even more murky when it is something we are working at turning on. :-)
Maybe we should think of having a separate sandboxing components for the code and its effects or a keyword adding, I'll raise at our meeting.

Comment 10

[Benyamin Limanto]

on nightly 92, scholar.google.com isn't affected with win32k lockdown, on 90 stable it did. It will crash even restored. See the crash report related to this bug

Comment 11

[Chris Peterson [:cpeterson]]

There are 20 crash reports from 2 installs of Firefox 92.0. They all have Fission enabled, "ContentSandboxWin32kState: Win32k Lockdown enabled", and locale ja.

Comment 12

[Bob Owen (:bobowen)]

Attachment: Bug 1713973 p2: Add Uniscribe Line Breaking via chromium-sandbox IPC. r=tkikuchi!,jfkthame!

Comment 13

[Bob Owen (:bobowen)]

Attachment: WIP Bug 1713973 p1: Add caching for calls to NS_GetComplexLineBreaks.

Comment 14

[Bob Owen (:bobowen)]

Attachment: WIP Bug 1713973 p2: Add Uniscribe Line Breaking via chromium-sandbox IPC.

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: revised-chunking.patch

Revised heuristic for chunk-overlap that has less risk of making errors near the boundary.

Comment 17

[Bob Owen (:bobowen)]

Attachment: Bug 1713973 p1: Add caching for calls to NS_GetComplexLineBreaks. r=jfkthame!

Comment 18

[Bob Owen (:bobowen)]

Attachment: Bug 1713973 p3: Use brokered complex line breaking when win32k lockdown is enabled. r=tkikuchi!,jfkthame!

Depends on D126809

Comment 19

[Bob Owen (:bobowen)]

Attachment: Bug 1713973 p4: Test brokered complex breaker against Uniscribe in content. r=jfkthame!

Depends on D129143

Comment 20

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=b215c163d7ed1ca94c22b403a497cfff63d93df8

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f4ebffa454c
p1: Add caching for calls to NS_GetComplexLineBreaks. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/d37fc64caa47
p2: Add Uniscribe Line Breaking via chromium-sandbox IPC. r=tkikuchi,jfkthame
https://hg.mozilla.org/integration/autoland/rev/45862297ac71
p3: Use brokered complex line breaking when win32k lockdown is enabled. r=jfkthame,tkikuchi
https://hg.mozilla.org/integration/autoland/rev/5d9a715cee85
p4: Test brokered complex breaker against Uniscribe in content. r=jfkthame

Comment 22

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/2f4ebffa454c
https://hg.mozilla.org/mozilla-central/rev/d37fc64caa47
https://hg.mozilla.org/mozilla-central/rev/45862297ac71
https://hg.mozilla.org/mozilla-central/rev/5d9a715cee85

Comment 23

[NN]

Is it fixed in the latest Firefox Developer Edition ?
I still get an exception with gpu.sandbox set.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Can you confirm that you've got version 95.0b1?

Comment 25

[NN]

Yes.
The issue reproduces with the following settings:
security.sandbox.content.level 20
security.sandbox.gpu.level 1

Navigate to https://addons.mozilla.org/

Comment 26

[Bob Owen (:bobowen)]

(In reply to NN from comment #25)

Yes.
The issue reproduces with the following settings:
security.sandbox.content.level 20
security.sandbox.gpu.level 1

Navigate to https://addons.mozilla.org/

It's possible that you are still hitting a related problem, but this one was specifically to do with win32k lockdown.
It's great that you are testing these other settings though, please would you file a new bug (I'm guessing that https://addons.mozilla.org/ might trigger something for you because of your locale, for the steps to reproduce, it would be best if you used a page that doesn't depend on locale to trigger the issue, to make it easier to test).
Also any crash report links would be very useful.