1718348 - Crash in [@ mozilla::RandomUint64OrDie]

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Andrew McCreight [:mccr8]]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/9315bd66-1b12-448a-8c2e-fb1ec0210624

Reason: EXCEPTION_BREAKPOINT

Top 9 frames of crashing thread:

0 mozglue.dll mozilla::RandomUint64OrDie mfbt/RandomNum.cpp:156
1 xul.dll static mozilla::ipc::NodeController::InitChildProcess ipc/glue/NodeController.cpp:645
2 xul.dll ChildThread::Init ipc/chromium/src/chrome/common/child_thread.cc:42
3 xul.dll base::Thread::ThreadMain ipc/chromium/src/base/thread.cc:181
4 xul.dll `anonymous namespace'::ThreadFunc ipc/chromium/src/base/platform_thread_win.cc:19
5 kernel32.dll BaseThreadInitThunk 
6 mozglue.dll patched_BaseThreadInitThunk mozglue/dllservices/WindowsDllBlocklist.cpp:592
7 ntdll.dll RtlUserThreadStart 
8 kernelbase.dll TerminateProcessOnMemoryExhaustion 

This is likely a regression from Nika's recent IPC work, but it apparently relates to sandboxing, judging by the comment on this crash: "Fission enabled; security.sandbox.content.level = 20 triggers this crash.
Previous version (Nightly 90) didn't have this problem. 2 weeks ago there was no problem with this setting."

Comment 1

[Andrew McCreight [:mccr8]]

Probably not worth tracking because it requires some weird sandbox setting (?).

Comment 2

[Andrew McCreight [:mccr8]]

Also possibly interesting that there's no annotation in the crash report for the release assert.

Comment 3

[Andrew McCreight [:mccr8]]

Bob, have you seen sandboxing issues with whatever RNG syscall we're using here before? Should we worry about causing issues with the sandboxing level set to 20, whatever that means? Thanks.

Comment 4

[donqk]

(In reply to Andrew McCreight [:mccr8] from comment #3)

Bob, have you seen sandboxing issues with whatever RNG syscall we're using here before? Should we worry about causing issues with the sandboxing level set to 20, whatever that means? Thanks.

Sandboxing level, defined in "mozilla-central/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp" line 546 sets the integrity level to untrusted, and that's the only difference with the default level "6", that sets the integrity level to low.

I submitted that crash report, i've been testing that setting for months, and never had a problem. I guess is not used in testing, so this error didnt pop before.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1706374

Comment 6

[Andrew McCreight [:mccr8]]

This requires an unusual pref setting.

Comment 7

[Chris Peterson [:cpeterson]]

This doesn't look like a Fission crash even though the crash report in comment 0 has DOMFissionEnabled=1. Adding [not-a-fission-bug] whiteboard tag to hide this bug from Fission bug triage queries.

Comment 8

[Bob Owen (:bobowen)]

(In reply to donqk from comment #4)

(In reply to Andrew McCreight [:mccr8] from comment #3)

Bob, have you seen sandboxing issues with whatever RNG syscall we're using here before? Should we worry about causing issues with the sandboxing level set to 20, whatever that means? Thanks.

Sandboxing level, defined in "mozilla-central/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp" line 546 sets the integrity level to untrusted, and that's the only difference with the default level "6", that sets the integrity level to low.

I submitted that crash report, i've been testing that setting for months, and never had a problem. I guess is not used in testing, so this error didnt pop before.

Thanks for submitting the report.
The original idea around having a level 20 was this was similar to chromium (at the time) and so that we had plenty of "levels" to use in between to gradually increases restrictions.
As it happens we've started to use more separate prefs for things, mainly because they've become easier to use.
Anyway, we do want to start looking at getting to level 20 (which mainly means untrusted integrity and a USER_LOCKDOWN access token level) fairly soon, so this is something we'll need to address.

Comment 9

[Bob Owen (:bobowen)]

Just going to block this main sandboxing bug for now.

Comment 10

[Toshihito Kikuchi [:toshi]]

I believe this is the same issue as bug 1720746. The root cause is that loading bcryptPrimitives.dll fails in a thread with a restricted token, thus RtlGenRandom fails. Usually our process pre-loads bcryptPrimitives.dll in the main thread which has impersonation token, but if a new thread is spawned for some reason before pre-load, this happens. The fix for bug 1720746 is limited to ASan + Plugin process, but this bug implies the issue can happen on the content process without ASan. We need to remove that limitation.

Comment 11

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1718348 - Always preload bcryptPrimitives.dll in a sandboxed process. r=bobowen

Bug 1720746 was a test failure in the plugin process with ASan, but bug 1718348
tells us the same problem happens in the content process without ASan.
This patch expands the fix to all sandbox processes.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49cb7daae12c
Always preload bcryptPrimitives.dll in a sandboxed process.  r=bobowen

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/49cb7daae12c