1459335 - Repeated failed function hook attempts cause exhaustion of interceptor trampoline space

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-075ea3bc-7eb4-4b01-8703-679530180504.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyInProcess, 128>::GetNextTrampoline mozglue/misc/interceptor/VMSharingPolicies.h:38
1 xul.dll mozilla::interceptor::WindowsDllDetourPatcher<mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyInProcess, 128> >::CreateTrampoline mozglue/misc/interceptor/PatcherDetour.h:402
2 xul.dll mozilla::interceptor::WindowsDllDetourPatcher<mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyInProcess, 128> >::AddHook mozglue/misc/interceptor/PatcherDetour.h:166
3 xul.dll mozilla::interceptor::WindowsDllInterceptor<mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyInProcess, 128> >::AddHook mozglue/misc/nsWindowsDllInterceptor.h:192
4 xul.dll mozilla::plugins::BasicFunctionHook<15, int >::Register dom/plugins/ipc/FunctionHook.h:168
5 xul.dll mozilla::plugins::PluginModuleChild::AllocPPluginInstanceChild dom/plugins/ipc/PluginModuleChild.cpp:1821
6 xul.dll mozilla::plugins::PPluginModuleChild::OnMessageReceived ipc/ipdl/PPluginModuleChild.cpp:622
7 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage ipc/glue/MessageChannel.cpp:2141
8 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW ipc/glue/MessageChannel.cpp:2071
9 xul.dll mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1917

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

crashes with this signature are newly appearing in firefox 61 on windows, around half of them in the plugin process.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

The main issue that I am seeing in these dumps is that the interceptor's AddHook method is failing, but BasicFunctionHook doesn't distinguish between "we haven't hooked this yet" and "we tried to hook this but failed."

Every time a new plugin instance is created, we re-attempt to set the hook. Each attempt fails, but each attempt also consumes a trampoline, until the trampoline space is eventually exhausted.

While adding a more sophisticated allocator for interceptor trampolines could fix that, I think that would be overkill given the nature of the problem, and is better addressed at the BasicFunctionHook level.

ni? over to David to see what he thinks.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

I have created bug 1460002 as a meta to track similar problems in other areas of the code. Non-plugin crashes with the same signature will be fixed there.

Comment 3

[Jim Mathies [:jimm]]

FYI, david is on pto. He'll be back next week.

Comment 4

[David Parks [:handyman]]

Attachment: Distinguish between unregistered and failed DLL function interceptions

This just does the straight-forward thing of distinguishing between having never attempted to register vs having failed to register.

I had also wanted to move the initialization of the interceptions (the call to HookFunctions) from PluginModuleChild::AllocPPluginInstanceChild since, fundamentally, the mistake is that we keep trying to initialize every time we need a new PPluginInstance but the quirks system looks like it got mangled in the e10s move so we are doing some sketchy stuff about re-initialization that I _think_ is all moot anyway but, in the end, I decided it's not important enough to risk bothering.  Still, this implementation is kind of a cop-out.

---

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac1abcb7feece928d541cd08a9572d8a3ed92813

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f9400fe51c4
Distinguish between unregistered and failed DLL function interceptions. r=aklotz

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2f9400fe51c4