1347867 - Crash in CrashReporter::OOPInit (Quick Heal Antivirus SCDETOUR.DLL)

Status: NEW

(External Software Affecting Firefox :: Other, defect, P3)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-a316ec32-9ff6-4713-b6a5-7398a2170315.
=============================================================

Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	CrashReporter::OOPInit() 	toolkit/crashreporter/nsExceptionHandler.cpp:3558
1 	xul.dll 	mozilla::ipc::GeckoChildProcessHost::PrepareLaunch() 	ipc/glue/GeckoChildProcessHost.cpp:326
2 	xul.dll 	mozilla::ipc::GeckoChildProcessHost::AsyncLaunch(std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, base::ProcessArchitecture) 	ipc/glue/GeckoChildProcessHost.cpp:395
3 	xul.dll 	mozilla::gfx::GPUProcessHost::Launch() 	gfx/ipc/GPUProcessHost.cpp:44
4 	xul.dll 	mozilla::gfx::GPUProcessManager::LaunchGPUProcess() 	gfx/ipc/GPUProcessManager.cpp:141
5 	xul.dll 	gfxPlatform::Init() 	gfx/thebes/gfxPlatform.cpp:694
6 	xul.dll 	gfxPlatform::GetPlatform() 	gfx/thebes/gfxPlatform.cpp:535
7 	xul.dll 	mozilla::widget::GfxInfo::GetDWriteEnabled(bool*) 	widget/windows/GfxInfo.cpp:69
8 	xul.dll 	XPTC__InvokebyIndex 	xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64.asm:97
9 		@0x5208a39337 	
10 	xul.dll 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	js/xpconnect/src/XPCWrappedNative.cpp:1296
11 	xul.dll 	XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1019
12 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:460
13 	xul.dll 	CallGetter 	js/src/vm/NativeObject.cpp:1812
14 	xul.dll 	js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) 	js/src/jsobj.h:857
15 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:2794
16 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:406
17 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:478
18 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:2956
19 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:406
20 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:478
21 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:2956
22 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:406
23 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:478
24 	xul.dll 	CallGetter 	js/src/vm/NativeObject.cpp:1812
25 	xul.dll 	js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) 	js/src/vm/NativeObject.h:1510
26 	xul.dll 	js::Wrapper::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) 	js/src/proxy/Wrapper.cpp:143
27 	xul.dll 	GetPropertyOperation 	js/src/vm/Interpreter.cpp:192
28 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:2673
29 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:406
30 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:478
31 	xul.dll 	JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp:2788
32 	xul.dll 	nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) 	js/xpconnect/src/XPCWrappedJSClass.cpp:1213
33 	xul.dll 	nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) 	js/xpconnect/src/XPCWrappedJS.cpp:613
34 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/md/win32/xptcstubs_x86_64.cpp:174
35 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/md/win32/xptcstubs_asm_x86_64.asm:57
36 	xul.dll 	NS_CreateServicesFromCategory(char const*, nsISupports*, char const*, char16_t const*) 	xpcom/components/nsCategoryManager.cpp:821
37 	xul.dll 	nsXREDirProvider::DoStartup() 	toolkit/xre/nsXREDirProvider.cpp:1171
38 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:4292
39 	xul.dll 	XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4638
40 	xul.dll 	XRE_main(int, char** const, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4729
41 	firefox.exe 	NS_internal_main(int, char**, char**) 	browser/app/nsBrowserApp.cpp:305
42 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
43 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
44 	kernel32.dll 	BaseThreadInitThunk 	
45 	firefox.exe 	firefox.exe@0x607f 	
46 	firefox.exe 	firefox.exe@0x607f 	
47 	ntdll.dll 	RtlUserThreadStart

this crash signature is somewhat regressing in firefox 53 and later on windows (overall it's still rather low volume though), which is likely related to the introduction of the gpu process.

Comment 1

[Benjamin Smedberg]

MOZ_CRASH(can't start crash reporter server())

This being a release assert dates back to pretty ancient history, bug 516759. I don't know what conditions can cause us to not be able to start OOP crashreporting, but I'm skeptical that it should be a fatal error. Ted thoughts?

Comment 2

[u279076]

First started to show up in 53.0a1 with 20161205030204:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6bdef7ba8b4108a996b9f61ef9f81c5ea6c93017&tochange=bd9e81439725f3d4135652cc3d65f2bfba527b7b

I suspect bug 1319702 might be involved.

> likely related to the introduction of the gpu process.

This seems purely circumstantial.

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #1)
> This being a release assert dates back to pretty ancient history, bug
> 516759. I don't know what conditions can cause us to not be able to start
> OOP crashreporting, but I'm skeptical that it should be a fatal error. Ted
> thoughts?

I would guess that any process that hits that failure is likely to fail in some other way in the very near future, but we ought to be able to survive this, yes. We'd probably just need to change the child process spawning logic to ensure that it doesn't try to pass down crashreporter details if the crash reporter server bits have failed to initialize.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Fairly low volume crash, wontfix for 53.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Not high volume in 56 beta - 58 crashes in beta 8 but from 11 installations. 
So not a high priority (until it is).

Comment 6

[[:philipp]]

[Tracking Requested - why for this release]:
the signature seems to be correlated to 64bit browser versions - it's the top crashing signature in early data for 56.0b9 after we started migrating users on the beta channel.

Comment 7

[[:philipp]]

ni ping to make you aware of the issue

Comment 8

[Chris Peterson [:cpeterson]]

gsvelto or dvander, this GPU process crash has spiked in Beta 56 after we started automatically migrating 32-bit users to 64-bit Firefox. What do you recommend we do?

Comment 9

[Gerry Chang [:gchang]]

Track 56+ as top crash.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

2700 crashes in beta 9 so far, so a very high crash rate. Marking this as a release blocker.

Comment 11

[[:philipp]]

correlation data that's now available for the signature show, this crash might be triggered by 3rd party software "quickheal antivirus"
(77.48% in signature vs 00.48% overall) Module "SCDETOUR.DLL"
(21.85% in signature vs 00.21% overall) Module "ScDetour.Dll"

Comment 12

[[:philipp]]

i can reproduce the startup crash with 64bit firefox once i install their trial version out of the box.

Comment 13

[Chris Peterson [:cpeterson]]

Marco, can we block SCDETOUR.DLL?

Comment 14

[David Anderson [:dvander] - inactive, e-mail if emergency]

It might be worth doing a very quick diagnostic patch to see which failure case we are actually hitting here [1].

This crash isn't specific to the GPU process launching - I also see content process launches in the stack. Though usually the GPU process launches first so it would be the first thing to encounter this error. I agree with Ted that even if we make this a "soft" failure, probably it's a portent of bad things to come soon after. 

[1] http://searchfox.org/mozilla-central/source/toolkit/crashreporter/breakpad-client/linux/crash_generation/crash_generation_server.cc#85

Comment 15

[Chris Peterson [:cpeterson]]

Bob, can you take a look at this antivirus DLL crash or recommend someone else? Is there anything we can do to work around the crash without blocking the DLL?

Comment 16

[(not currently active) Ted Mielczarek]

(In reply to David Anderson [:dvander] from comment #14)
> It might be worth doing a very quick diagnostic patch to see which failure
> case we are actually hitting here [1].

FYI, these crashes are on Windows, so this is the code in question:
https://dxr.mozilla.org/mozilla-central/rev/37b95547f0d27565452136d16b2df2857be840f6/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc#212

It's calling CreateMutex, CreateEvent, RegisterWaitForSingleObject, CreateNamedPipe, and SetEvent. If any of those fail it returns false. That's pretty basic Win32 API stuff.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

I'm ok with just blocking this DLL for now to see if that works.

Comment 18

[Bob Owen (:bobowen)]

(In reply to Chris Peterson [:cpeterson] from comment #15)
> Bob, can you take a look at this antivirus DLL crash or recommend someone
> else? Is there anything we can do to work around the crash without blocking
> the DLL?

This isn't really something I've dealt with that much.
It's the CreateNamedPipe call that is failing.
That appears to be failing because ntdll!NtQueryInformationToken and KERNELBASE!_imp_NtCreateNamedPipeFile are hooked (jump into Scdetour!DllRegisterServer).

I think it is the second one in particular that is failing.
The last errors are:
LastErrorValue: (Win32) 0x57 (87) - The parameter is incorrect.
LastStatusValue: (NTSTATUS) 0xc000000d - An invalid parameter was passed to a service or function.

I think dmajor, aklotz and ccorcoran have more experience with this sort of thing.

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

dmajor, can you give this a try?

Comment 20

[Julien Cristau [:jcristau]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #17)
> I'm ok with just blocking this DLL for now to see if that works.

Tried that (adding scdetour.dll to the blocklist), didn't seem to work.

Comment 21

[Marco Castelluccio [:marco]]

(In reply to Chris Peterson [:cpeterson] from comment #13)
> Marco, can we block SCDETOUR.DLL?

Looks like we can't. Needinfoing aklotz as well to see if he can take a look at the underlying reason why we can't block them.

(In reply to David Anderson [:dvander] from comment #14)
> This crash isn't specific to the GPU process launching - I also see content
> process launches in the stack. Though usually the GPU process launches first
> so it would be the first thing to encounter this error. I agree with Ted
> that even if we make this a "soft" failure, probably it's a portent of bad
> things to come soon after.

Since philipp can reproduce, perhaps we can check if this is actually true.

Comment 22

[[:philipp]]

Attachment: screenshot.png

turning that 'MOZ_CRASH(can't start crash reporter server())' into a MOZ_ASSERT in a local build produced a windows error report in xul.dll (instead of the breakpad reporter) when trying to launch the browser. afterwards the browser window opened up but was non-functional & no sites could be loaded. 

attaching a screenshot with the output of the browser console.

Comment 23

[Chris Peterson [:cpeterson]]

astevenson has notified Quick Heal support and they will look into the crash.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Keep in mind we have a related startup crash in bug 1397750 as we test to see if blocklisting works.

Comment 25

[Chris Peterson [:cpeterson]]

Bob says: "ScDetour.Dll is already loaded as soon as I open the Firefox executable in windbg, so definitely before we initialize our blocklist." 

I think we might be seeing a similar problem in bug 1369361 where blocklisted Lenovo DLLs still being loaded.

Comment 26

[Chris Peterson [:cpeterson]]

(In reply to Bob Owen (:bobowen) from comment #18)
> This isn't really something I've dealt with that much.
> It's the CreateNamedPipe call that is failing.
> That appears to be failing because ntdll!NtQueryInformationToken and
> KERNELBASE!_imp_NtCreateNamedPipeFile are hooked (jump into
> Scdetour!DllRegisterServer).
> 
> I think it is the second one in particular that is failing.
> The last errors are:
> LastErrorValue: (Win32) 0x57 (87) - The parameter is incorrect.
> LastStatusValue: (NTSTATUS) 0xc000000d - An invalid parameter was passed to
> a service or function.

Is there anything suspicious about the parameters we are passing to CreateNamedPipe() that might be causing scdetour.dll's hook to return an invalid parameter error? Since scdetour.dll is returning an error and not itself crashing, can we work around the error?

Comment 27

[Jim Mathies [:jimm]]

I've been looking for a way to reproduce the crash but unfortunately no luck. Test steps:

Window 10, 64-bit

1) Installed firefox 32-bit in the 64-bit install location
2) Installed the full quick heal suite 
3) run 32-bit firefox (success)
4) download 64-bit firefox installer
5) upgrade firefox 32-bit to 64-bit
6) run Firefox 64-bit (success)

Comment 28

[[:philipp]]

try to enable the browser sandbox in the internet & network settings of the software.

Comment 29

[Bob Owen (:bobowen)]

(In reply to Chris Peterson [:cpeterson] from comment #26)
> (In reply to Bob Owen (:bobowen) from comment #18)
> > This isn't really something I've dealt with that much.
> > It's the CreateNamedPipe call that is failing.
> > That appears to be failing because ntdll!NtQueryInformationToken and
> > KERNELBASE!_imp_NtCreateNamedPipeFile are hooked (jump into
> > Scdetour!DllRegisterServer).
> > 
> > I think it is the second one in particular that is failing.
> > The last errors are:
> > LastErrorValue: (Win32) 0x57 (87) - The parameter is incorrect.
> > LastStatusValue: (NTSTATUS) 0xc000000d - An invalid parameter was passed to
> > a service or function.
> 
> Is there anything suspicious about the parameters we are passing to
> CreateNamedPipe() that might be causing scdetour.dll's hook to return an
> invalid parameter error? Since scdetour.dll is returning an error and not
> itself crashing, can we work around the error?

I don't think so.

The actual syscall looks like this (parameter wise):
NTSYSAPI 
NTSTATUS
NTAPI
NtCreateNamedPipeFile(
  OUT PHANDLE             NamedPipeFileHandle,
  IN ACCESS_MASK          DesiredAccess,
  IN POBJECT_ATTRIBUTES   ObjectAttributes,
  OUT PIO_STATUS_BLOCK    IoStatusBlock,
  IN ULONG                ShareAccess,
  IN ULONG                CreateDisposition,
  IN ULONG                CreateOptions,
  IN BOOLEAN              WriteModeMessage,
  IN BOOLEAN              ReadModeMessage,
  IN BOOLEAN              NonBlocking,
  IN ULONG                MaxInstances,
  IN ULONG                InBufferSize,
  IN ULONG                OutBufferSize,
  IN PLARGE_INTEGER       DefaultTimeOut );

It look like they're messing up the WriteModeMessage and ReadModeMessage BOOLEANs during their hook.
However, even if I correct them just prior to the actual syscall, the pipe creation works, but you still end up with a non-functioning browser.

I guess if they fixed that problem we might be able to block the loading in the child with the chromium sandbox DLL blocking.

Comment 30

[Bob Owen (:bobowen)]

(In reply to Bob Owen (:bobowen) from comment #29)
...
> It look like they're messing up the WriteModeMessage and ReadModeMessage
> BOOLEANs during their hook.

I think this is because in the assembly, where it is copying the parameters from further up the stack, it's only moving the lower 8 bits into the BOOLEANs instead of 32 bits.

Comment 31

[Liz Henry (:lizzard) (relman/hg->git project)]

Meanwhile - an engineer from QuickHeal has written back to say they fixed the issue and are rolling out the new version.

Comment 32

[Liz Henry (:lizzard) (relman/hg->git project)]

Release Note Request (optional, but appreciated)
[Why is this notable]:
[Affects Firefox for Android]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:

Given that they are already rolling out their fix, the decreased crash volume on beta versions after beta 9, and the greater % of total users on beta who use QuickHeal than the % on release, I think we don't need to count this as a release blocker. Some users will still crash, so we may want to add this as a known issue in release notes for older versions of QH.

Comment 33

[Chris Peterson [:cpeterson]]

[Why is this notable]:
Windows users running 64-bit Firefox and an old version of the Quick Heal security software will hit startup crashes. We plan to automatically migrate some 32-bit Firefox users to 64-bit during the 56 release, so some users may hit this crash but not know they are now running 64-bit Firefox.
 
[Affects Firefox for Android]:
No

[Suggested wording]:
Some versions of the Quick Heal security software can cause 64-bit Firefox startup crashes on Windows. Quick Heal has fixed the crash in the latest version of their software. To fix this crash, please install the latest Quick heal update or [re-install 32-bit Firefox](https://www.mozilla.org/firefox/all/).

[Links (documentation, blog post, etc)]:
https://www.mozilla.org/firefox/all/

Comment 34

[Chris Peterson [:cpeterson]]

I still see 120+ crash reports from 56.0 and 56.0.1 with scdetour.dll versions 3.0.0.193 and 3.0.1.24. I'm checking with Quick Heal to see which DLL version has their fix.

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with
"Some third party software (Comodo Internet Security, Kaspersky, Quick Heal Antivirus) are known to cause issues with Firefox 64-bit. These vendors have published new releases which addresses the issues."
as wording

Comment 36

[Marco Castelluccio [:marco]]

(In reply to Chris Peterson [:cpeterson] from comment #34)
> I still see 120+ crash reports from 56.0 and 56.0.1 with scdetour.dll
> versions 3.0.0.193 and 3.0.1.24. I'm checking with Quick Heal to see which
> DLL version has their fix.

Did they reply?

Comment 37

[Chris Peterson [:cpeterson]]

Yes. Quick Heal says the crash should be fixed in scdetour.dll version 3.0.1.25.

Comment 38

[Liz Henry (:lizzard) (relman/hg->git project)]

While there are still a few crashes remaining the volume is very low.

Comment 39

[Liz Henry (:lizzard) (relman/hg->git project)]

A few of these crashes are now showing up in beta 64.

Comment 40

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

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.