Closed Bug 1444699 Opened 6 years ago Closed 6 years ago

Crash in sandbox::`anonymous namespace'::WarmupWindowsLocales

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: jseward, Assigned: bobowen)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is
report bp-3538bf15-297c-4d7f-a210-1e7e80180310.
=============================================================

Top 10 frames of crashing thread:

0  @0x340 
1 firefox.exe sandbox::`anonymous namespace'::WarmupWindowsLocales security/sandbox/chromium/sandbox/win/src/target_services.cc:115
2 firefox.exe sandbox::TargetServicesBase::LowerToken security/sandbox/chromium/sandbox/win/src/target_services.cc:155
3 xul.dll mozilla::dom::ContentChild::RecvSetProcessSandbox dom/ipc/ContentChild.cpp:1719
4 xul.dll mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:5635
5 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage ipc/glue/MessageChannel.cpp:2133
6 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW ipc/glue/MessageChannel.cpp:2063
7 xul.dll mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1909
8 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1942
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1040

=============================================================

This is topcrash #5 in the Windows nightly 20180308100121.  There are 
actually only two different installations involved; on the other hand
it looks like it might be actionable.
Flags: needinfo?(jld)
Looks like this must be Windows only, so I'll take a look at this.
Flags: needinfo?(jld) → needinfo?(bobowencode)
From the code (and assembly) either GetUserDefaultLocaleName_func is not getting initialised correctly (which seems unlikely) or GetProcAddress is returning a weird value, so I imagine that's the culprit.
There are third party hooking DLLs loaded, so perhaps they're messing this up somehow.

It's only from 3 installations and one build id, but as you say looks like now we don't have WinXP we can just link to the function directly.

Given the current low level of this I don't think it's worth us having a patch to the chromium code, but setting to P2 because patching upstream seems so simple.
Flags: needinfo?(bobowencode)
Priority: -- → P2
Updating signature, because we have had slightly different spikes before.
Crash Signature: [@ sandbox::`anonymous namespace'::WarmupWindowsLocales] → [@ WarmupWindowsLocales]
Working on the chromium patch now.
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
This was only required because it is not available on Windows XP, which is no
longer supported. Patch already landed upstream in chromium.
Attachment #8970132 - Flags: review?(davidp99)
Priority: P2 → P1
This was only required because it is not available on Windows XP, which is no
longer supported. Patch already landed upstream in chromium.

Just realised that I forgot to update security/sandbox/chromium-shim/patches/after_update/patch_order.txt in that last patch.
Attachment #8970134 - Flags: review?(davidp99)
Attachment #8970132 - Attachment is obsolete: true
Attachment #8970132 - Flags: review?(davidp99)
Comment on attachment 8970134 [details] [diff] [review]
Remove dynamic load and call for GetUserDefaultLocaleName

Huh.  I wonder why this was crashing in the first place.
Attachment #8970134 - Flags: review?(davidp99) → review+
(In reply to David Parks (dparks) [:handyman] from comment #9)

Thanks David.

> Huh.  I wonder why this was crashing in the first place.

Indeed, although James Forshaw noticed in the review for chromium that GetUserDefaultLocaleName_func was being called with length in bytes when it should have been length in characters.
Given that we were passing a LOCALE_NAME_MAX_LENGTH buffer anyway, it probably didn't matter, but maybe in some builds, on some machines there's an issue further down that resulted in this.
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/86f3ef9d3d24
Remove dynamic load and call for GetUserDefaultLocaleName. r=handyman
https://hg.mozilla.org/mozilla-central/rev/86f3ef9d3d24
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.