Closed
Bug 1444699
Opened 6 years ago
Closed 6 years ago
Crash in sandbox::`anonymous namespace'::WarmupWindowsLocales
Categories
(Core :: Security: Process Sandboxing, defect, P1)
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)
6.79 KB,
patch
|
handyman
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(jld)
Assignee | ||
Comment 1•6 years ago
|
||
Looks like this must be Windows only, so I'll take a look at this.
Flags: needinfo?(jld) → needinfo?(bobowencode)
Assignee | ||
Comment 2•6 years ago
|
||
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
Assignee | ||
Comment 3•6 years ago
|
||
I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=821290
Assignee | ||
Comment 4•6 years ago
|
||
Updating signature, because we have had slightly different spikes before.
Crash Signature: [@ sandbox::`anonymous namespace'::WarmupWindowsLocales] → [@ WarmupWindowsLocales]
Assignee | ||
Comment 5•6 years ago
|
||
Working on the chromium patch now.
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•6 years ago
|
||
https://chromium-review.googlesource.com/c/chromium/src/+/1012119
Assignee | ||
Comment 7•6 years ago
|
||
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)
Assignee | ||
Updated•6 years ago
|
Priority: P2 → P1
Assignee | ||
Comment 8•6 years ago
|
||
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)
Assignee | ||
Updated•6 years ago
|
Attachment #8970132 -
Attachment is obsolete: true
Attachment #8970132 -
Flags: review?(davidp99)
Comment 9•6 years ago
|
||
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+
Assignee | ||
Comment 11•6 years ago
|
||
(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.
Comment 12•6 years ago
|
||
Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/86f3ef9d3d24 Remove dynamic load and call for GetUserDefaultLocaleName. r=handyman
Comment 13•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/86f3ef9d3d24
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
status-firefox59:
--- → wontfix
status-firefox60:
--- → wontfix
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → wontfix
You need to log in
before you can comment on or make changes to this bug.
Description
•